Skip to content

fix(core): Prevents trying to trigger incremental hydration on CSR #58366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
thePunderWoman wants to merge 2 commits into from
Closed

fix(core): Prevents trying to trigger incremental hydration on CSR #58366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f86304
fix(core): Prevents trying to trigger incremental hydration on CSR
thePunderWoman Oct 25, 2024
2608df9
refactor(core): prevent unnecessary hydration work in csr cases
thePunderWoman Oct 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 82 additions & 57 deletions packages/core/src/defer/instructions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,19 @@ export const DEFER_BLOCK_CONFIG = new InjectionToken<DeferBlockConfig>(
);

/**
* Determines whether defer blocks should be fully rendered through on the server side
* for incremental hydration.
* Determines whether "hydrate" triggers should be activated. Triggers are activated in the following cases:
* - on the server, when incremental hydration is enabled, to trigger the block and render the main content
* - on the client for blocks that were server-side rendered, to start hydration process
*/
function shouldTriggerWhenOnServer(injector: Injector) {
return !isPlatformBrowser(injector) && isIncrementalHydrationEnabled(injector);
function shouldActivateHydrateTrigger(lView: LView, tNode: TNode): boolean {
const lDetails = getLDeferBlockDetails(lView, tNode);
const injector = lView[INJECTOR]!;
// TODO(incremental-hydration): ideally, this check should only happen once and then stored on
// LDeferBlockDetails as a flag. This would make subsequent lookups very cheap.
return (
isIncrementalHydrationEnabled(injector) &&
(!isPlatformBrowser(injector) || lDetails[SSR_UNIQUE_ID] !== null)
);
}

// TODO(incremental-hydration): Optimize this further by moving the calculation to earlier
Expand Down Expand Up @@ -347,9 +355,6 @@ export function ɵɵdeferWhen(rawValue: unknown) {
renderedState === DeferBlockState.Placeholder) &&
shouldTriggerWhenOnClient(lView[INJECTOR]!, lDetails, tDetails)
) {
// The `when` condition has changed to `true`, trigger defer block loading
// if the block is either in initial (nothing is rendered) or a placeholder
// state.
triggerDeferBlock(lView, tNode);
}
} finally {
Expand Down Expand Up @@ -392,18 +397,22 @@ export function ɵɵdeferPrefetchWhen(rawValue: unknown) {
*/
export function ɵɵdeferHydrateWhen(rawValue: unknown) {
const lView = getLView();
const tNode = getSelectedTNode();

if (!shouldActivateHydrateTrigger(lView, tNode)) {
return;
}

// TODO(incremental-hydration): audit all defer instructions to reduce unnecessary work by
// moving function calls inside their relevant control flow blocks
const bindingIndex = nextBindingIndex();
const tNode = getSelectedTNode();
const tView = getTView();
const hydrateTriggers = getHydrateTriggers(tView, tNode);
hydrateTriggers.set(DeferBlockTrigger.When, null);

if (bindingUpdated(lView, bindingIndex, rawValue)) {
const injector = lView[INJECTOR]!;
if (shouldTriggerWhenOnServer(injector)) {
if (!isPlatformBrowser(injector)) {
// We are on the server and SSR for defer blocks is enabled.
triggerDeferBlock(lView, tNode);
} else {
Expand Down Expand Up @@ -434,12 +443,14 @@ export function ɵɵdeferHydrateWhen(rawValue: unknown) {
export function ɵɵdeferHydrateNever() {
const lView = getLView();
const tNode = getCurrentTNode()!;
const hydrateTriggers = getHydrateTriggers(getTView(), tNode);
hydrateTriggers.set(DeferBlockTrigger.Never, null);
if (shouldActivateHydrateTrigger(lView, tNode)) {
const hydrateTriggers = getHydrateTriggers(getTView(), tNode);
hydrateTriggers.set(DeferBlockTrigger.Never, null);

if (shouldTriggerWhenOnServer(lView[INJECTOR]!)) {
// We are on the server and SSR for defer blocks is enabled.
triggerDeferBlock(lView, tNode);
if (!isPlatformBrowser(lView[INJECTOR]!)) {
// We are on the server and SSR for defer blocks is enabled.
triggerDeferBlock(lView, tNode);
}
}
}

Expand All @@ -466,14 +477,16 @@ export function ɵɵdeferPrefetchOnIdle() {
export function ɵɵdeferHydrateOnIdle() {
const lView = getLView();
const tNode = getCurrentTNode()!;
const hydrateTriggers = getHydrateTriggers(getTView(), tNode);
hydrateTriggers.set(DeferBlockTrigger.Idle, null);
if (shouldActivateHydrateTrigger(lView, tNode)) {
const hydrateTriggers = getHydrateTriggers(getTView(), tNode);
hydrateTriggers.set(DeferBlockTrigger.Idle, null);

if (shouldTriggerWhenOnServer(lView[INJECTOR]!)) {
// We are on the server and SSR for defer blocks is enabled.
triggerDeferBlock(lView, tNode);
} else {
scheduleDelayedHydrating(onIdle, lView, tNode);
if (!isPlatformBrowser(lView[INJECTOR]!)) {
// We are on the server and SSR for defer blocks is enabled.
triggerDeferBlock(lView, tNode);
} else {
scheduleDelayedHydrating(onIdle, lView, tNode);
}
}
}

Expand Down Expand Up @@ -524,22 +537,26 @@ export function ɵɵdeferPrefetchOnImmediate() {
export function ɵɵdeferHydrateOnImmediate() {
const lView = getLView();
const tNode = getCurrentTNode()!;
const injector = lView[INJECTOR]!;
const lDetails = getLDeferBlockDetails(lView, tNode);
const hydrateTriggers = getHydrateTriggers(getTView(), tNode);
hydrateTriggers.set(DeferBlockTrigger.Immediate, null);
if (shouldActivateHydrateTrigger(lView, tNode)) {
const injector = lView[INJECTOR]!;
const lDetails = getLDeferBlockDetails(lView, tNode);
const hydrateTriggers = getHydrateTriggers(getTView(), tNode);
hydrateTriggers.set(DeferBlockTrigger.Immediate, null);

if (shouldTriggerWhenOnServer(injector)) {
triggerDeferBlock(lView, tNode);
} else {
incrementallyHydrateFromBlockName(
injector,
lDetails[SSR_UNIQUE_ID]!,
(deferBlock: DeferBlock) => triggerAndWaitForCompletion(deferBlock),
);
if (!isPlatformBrowser(injector)) {
triggerDeferBlock(lView, tNode);
} else {
// TODO(incremental-hydration): see if we can resolve the circular dep issue
// that required passing cleanup fns via the 3rd param here. Ideally we could
// move the `triggerAndWaitForCompletion` call to a better location.
incrementallyHydrateFromBlockName(
injector,
lDetails[SSR_UNIQUE_ID]!,
(deferBlock: DeferBlock) => triggerAndWaitForCompletion(deferBlock),
Copy link
Copy Markdown
Contributor

@AndrewKushnir AndrewKushnir Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd propose adding a TODO here to refactor the code (resolving circular deps if needed), so that we don't have to have this callback function (it should probably be inside of the incrementallyHydrateFromBlockName one).

Copy link
Copy Markdown
Contributor Author

@thePunderWoman thePunderWoman Oct 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initial circular dep problem is actually why this function is here at all. It was initially in the proposed location and wasn't resolvable without extracting this into a function like this. These are the cleanup fns. I can add a TODO though.

);
}
}
}

/**
* Creates runtime data structures for the `on timer` deferred trigger.
* @param delay Amount of time to wait before loading the content.
Expand All @@ -566,14 +583,16 @@ export function ɵɵdeferPrefetchOnTimer(delay: number) {
export function ɵɵdeferHydrateOnTimer(delay: number) {
const lView = getLView();
const tNode = getCurrentTNode()!;
const hydrateTriggers = getHydrateTriggers(getTView(), tNode);
hydrateTriggers.set(DeferBlockTrigger.Timer, delay);
if (shouldActivateHydrateTrigger(lView, tNode)) {
const hydrateTriggers = getHydrateTriggers(getTView(), tNode);
hydrateTriggers.set(DeferBlockTrigger.Timer, delay);

if (shouldTriggerWhenOnServer(lView[INJECTOR]!)) {
// We are on the server and SSR for defer blocks is enabled.
triggerDeferBlock(lView, tNode);
} else {
scheduleDelayedHydrating(onTimer(delay), lView, tNode);
if (!isPlatformBrowser(lView[INJECTOR]!)) {
// We are on the server and SSR for defer blocks is enabled.
triggerDeferBlock(lView, tNode);
} else {
scheduleDelayedHydrating(onTimer(delay), lView, tNode);
}
}
}

Expand Down Expand Up @@ -636,12 +655,14 @@ export function ɵɵdeferPrefetchOnHover(triggerIndex: number, walkUpTimes?: num
export function ɵɵdeferHydrateOnHover() {
const lView = getLView();
const tNode = getCurrentTNode()!;
const hydrateTriggers = getHydrateTriggers(getTView(), tNode);
hydrateTriggers.set(DeferBlockTrigger.Hover, null);
if (shouldActivateHydrateTrigger(lView, tNode)) {
const hydrateTriggers = getHydrateTriggers(getTView(), tNode);
hydrateTriggers.set(DeferBlockTrigger.Hover, null);

if (shouldTriggerWhenOnServer(lView[INJECTOR]!)) {
// We are on the server and SSR for defer blocks is enabled.
triggerDeferBlock(lView, tNode);
if (!isPlatformBrowser(lView[INJECTOR]!)) {
// We are on the server and SSR for defer blocks is enabled.
triggerDeferBlock(lView, tNode);
}
}
// The actual triggering of hydration on hover is handled by JSAction in
// event_replay.ts.
Expand Down Expand Up @@ -706,12 +727,14 @@ export function ɵɵdeferPrefetchOnInteraction(triggerIndex: number, walkUpTimes
export function ɵɵdeferHydrateOnInteraction() {
const lView = getLView();
const tNode = getCurrentTNode()!;
const hydrateTriggers = getHydrateTriggers(getTView(), tNode);
hydrateTriggers.set(DeferBlockTrigger.Interaction, null);
if (shouldActivateHydrateTrigger(lView, tNode)) {
const hydrateTriggers = getHydrateTriggers(getTView(), tNode);
hydrateTriggers.set(DeferBlockTrigger.Interaction, null);

if (shouldTriggerWhenOnServer(lView[INJECTOR]!)) {
// We are on the server and SSR for defer blocks is enabled.
triggerDeferBlock(lView, tNode);
if (!isPlatformBrowser(lView[INJECTOR]!)) {
// We are on the server and SSR for defer blocks is enabled.
triggerDeferBlock(lView, tNode);
}
}
// The actual triggering of hydration on interaction is handled by JSAction in
// event_replay.ts.
Expand Down Expand Up @@ -776,13 +799,15 @@ export function ɵɵdeferPrefetchOnViewport(triggerIndex: number, walkUpTimes?:
export function ɵɵdeferHydrateOnViewport() {
const lView = getLView();
const tNode = getCurrentTNode()!;
const hydrateTriggers = getHydrateTriggers(getTView(), tNode);
hydrateTriggers.set(DeferBlockTrigger.Viewport, null);
const injector = lView[INJECTOR]!;
if (shouldActivateHydrateTrigger(lView, tNode)) {
const hydrateTriggers = getHydrateTriggers(getTView(), tNode);
hydrateTriggers.set(DeferBlockTrigger.Viewport, null);
const injector = lView[INJECTOR]!;

if (shouldTriggerWhenOnServer(injector)) {
// We are on the server and SSR for defer blocks is enabled.
triggerDeferBlock(lView, tNode);
if (!isPlatformBrowser(injector)) {
// We are on the server and SSR for defer blocks is enabled.
triggerDeferBlock(lView, tNode);
}
}
// The actual triggering of hydration on viewport happens in incremental.ts,
// since these instructions won't exist for dehydrated content.
Expand Down
16 changes: 9 additions & 7 deletions packages/core/src/event_delegation_utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,17 @@ export const sharedMapFunction = (rEl: RElement, jsActionMap: Map<string, Set<El
};

export function removeListenersFromBlocks(blockNames: string[], injector: Injector) {
let blockList: Element[] = [];
const jsActionMap = injector.get(BLOCK_ELEMENT_MAP);
for (let blockName of blockNames) {
if (jsActionMap.has(blockName)) {
blockList = [...blockList, ...jsActionMap.get(blockName)!];
if (blockNames.length > 0) {
let blockList: Element[] = [];
const jsActionMap = injector.get(BLOCK_ELEMENT_MAP);
for (let blockName of blockNames) {
if (jsActionMap.has(blockName)) {
blockList = [...blockList, ...jsActionMap.get(blockName)!];
}
}
const replayList = new Set(blockList);
replayList.forEach(removeListeners);
}
const replayList = new Set(blockList);
replayList.forEach(removeListeners);
}

export const removeListeners = (el: Element) => {
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/hydration/blocks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,5 @@ export async function incrementallyHydrateFromBlockName(
// the hydration process has finished, which could result in problems
await whenStable(injector.get(ApplicationRef));
}
return Promise.resolve();
}
51 changes: 49 additions & 2 deletions packages/platform-server/test/incremental_hydration_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,17 @@ import {
ɵwhenStable as whenStable,
} from '@angular/core';

import {getAppContents, prepareEnvironmentAndHydrate, resetTViewsFor} from './dom_utils';
import {getAppContents, hydrate, prepareEnvironmentAndHydrate, resetTViewsFor} from './dom_utils';
import {getComponentRef, ssr, timeout} from './hydration_utils';
import {getDocument} from '@angular/core/src/render3/interfaces/document';
import {isPlatformServer} from '@angular/common';
import {withEventReplay, withIncrementalHydration} from '@angular/platform-browser';
import {
provideClientHydration,
withEventReplay,
withIncrementalHydration,
} from '@angular/platform-browser';
import {TestBed} from '@angular/core/testing';
import {PLATFORM_BROWSER_ID} from '@angular/common/src/platform_id';

describe('platform-server partial hydration integration', () => {
const originalWindow = globalThis.window;
Expand Down Expand Up @@ -1393,6 +1399,47 @@ describe('platform-server partial hydration integration', () => {
}, 100_000);
});

describe('client side navigation', () => {
beforeEach(() => {
TestBed.configureTestingModule({
providers: [
{provide: PLATFORM_ID, useValue: PLATFORM_BROWSER_ID},
provideClientHydration(withIncrementalHydration()),
],
});
});

it('should not try to hydrate in CSR only cases', async () => {
@Component({
standalone: true,
selector: 'app',
template: `
<main (click)="fnA()">
@defer (hydrate when true) {
<article>
defer block rendered!
<span id="test" (click)="fnB()">{{value()}}</span>
</article>
} @placeholder {
<span>Outer block placeholder</span>
}
</main>
`,
})
class SimpleComponent {
value = signal('start');
fnA() {}
fnB() {
this.value.set('end');
}
}
const fixture = TestBed.createComponent(SimpleComponent);
fixture.detectChanges();

expect(fixture.nativeElement.innerHTML).toContain('Outer block placeholder');
});
});

describe('cleanup', () => {
it('should cleanup partial hydration blocks appropriately', async () => {
@Component({
Expand Down