Skip to content

Commit

Permalink
refactor(core): Remove jsaction from element after handling the event. (
Browse files Browse the repository at this point in the history
#55549)

This also adds a test to make sure that the event contract is still listening to other events, especially in the case where we may want partial hydration in the future.

PR Close #55549
  • Loading branch information
iteriani authored and AndrewKushnir committed May 2, 2024
1 parent 8cabb7a commit 95bf0c8
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 3 deletions.
20 changes: 17 additions & 3 deletions packages/core/src/hydration/event_replay.ts
Expand Up @@ -15,12 +15,13 @@ import {

import {APP_BOOTSTRAP_LISTENER, ApplicationRef, whenStable} from '../application/application_ref';
import {APP_ID} from '../application/application_tokens';
import {Injector} from '../di';
import {ENVIRONMENT_INITIALIZER, Injector} from '../di';
import {inject} from '../di/injector_compatibility';
import {Provider} from '../di/interface/provider';
import {attachLViewId, readLView} from '../render3/context_discovery';
import {setDisableEventReplayImpl} from '../render3/instructions/listener';
import {TNode, TNodeType} from '../render3/interfaces/node';
import {RNode} from '../render3/interfaces/renderer_dom';
import {RElement, RNode} from '../render3/interfaces/renderer_dom';
import {CLEANUP, LView, TVIEW, TView} from '../render3/interfaces/view';
import {isPlatformBrowser} from '../render3/util/misc_utils';
import {unwrapRNode} from '../render3/util/view_utils';
Expand All @@ -34,6 +35,8 @@ declare global {
var ngContracts: {[key: string]: EventContract};
}

const JSACTION_ATTRIBUTE = 'jsaction';

/**
* Returns a set of providers required to setup support for event replay.
* Requires hydration to be enabled separately.
Expand All @@ -44,6 +47,17 @@ export function withEventReplay(): Provider[] {
provide: IS_EVENT_REPLAY_ENABLED,
useValue: true,
},
{
provide: ENVIRONMENT_INITIALIZER,
useValue: () => {
setDisableEventReplayImpl((el: RElement) => {
if (el.hasAttribute(JSACTION_ATTRIBUTE)) {
el.removeAttribute(JSACTION_ATTRIBUTE);
}
});
},
multi: true,
},
{
provide: APP_BOOTSTRAP_LISTENER,
useFactory: () => {
Expand Down Expand Up @@ -128,7 +142,7 @@ export function setJSActionAttribute(
const events = nativeElementToEvents.get(nativeElement) ?? [];
const parts = events.map((event) => `${event}:`);
if (parts.length > 0) {
nativeElement.setAttribute('jsaction', parts.join(';'));
nativeElement.setAttribute(JSACTION_ATTRIBUTE, parts.join(';'));
}
}
}
Expand Down
14 changes: 14 additions & 0 deletions packages/core/src/render3/instructions/listener.ts
Expand Up @@ -28,6 +28,18 @@ import {
loadComponentRenderer,
} from './shared';

/**
* Contains a reference to a function that disables event replay feature
* for server-side rendered applications. This function is overridden with
* an actual implementation when the event replay feature is enabled via
* `withEventReplay()` call.
*/
let disableEventReplayFn = (el: RElement) => {};

export function setDisableEventReplayImpl(fn: (el: RElement) => void) {
disableEventReplayFn = fn;
}

/**
* Adds an event listener to the current node.
*
Expand Down Expand Up @@ -169,6 +181,8 @@ export function listenerInternal(
? (_lView: LView) => eventTargetResolver(unwrapRNode(_lView[tNode.index]))
: tNode.index;

disableEventReplayFn(native);

// In order to match current behavior, native DOM event listeners must be added for all
// events (including outputs).

Expand Down
Expand Up @@ -923,6 +923,9 @@
{
"name": "diPublicInInjector"
},
{
"name": "disableEventReplayFn"
},
{
"name": "elementPropertyInternal"
},
Expand Down
3 changes: 3 additions & 0 deletions packages/core/test/bundling/todo/bundle.golden_symbols.json
Expand Up @@ -809,6 +809,9 @@
{
"name": "diPublicInInjector"
},
{
"name": "disableEventReplayFn"
},
{
"name": "enterDI"
},
Expand Down
37 changes: 37 additions & 0 deletions packages/platform-server/test/event_replay_spec.ts
Expand Up @@ -147,6 +147,43 @@ describe('event replay', () => {
expect(clickSpy).toHaveBeenCalled();
});

it('should remove jsaction attributes, but continue listening to events.', async () => {
@Component({
standalone: true,
selector: 'app',
template: `
<div (click)="onClick()" id="1">
<div (click)="onClick()" id="2"></div>
</div>
`,
})
class SimpleComponent {
onClick() {}
}

const docContents = `<html><head></head><body>${EVENT_DISPATCH_SCRIPT}<app></app></body></html>`;
const html = await ssr(SimpleComponent, {doc: docContents});
const ssrContents = getAppContents(html);
const removeEventListenerSpy = spyOn(
document.body,
'removeEventListener',
).and.callThrough();
render(doc, ssrContents);
const el = doc.getElementById('1')!;
expect(el.hasAttribute('jsaction')).toBeTrue();
expect((el.firstChild as Element).hasAttribute('jsaction')).toBeTrue();
resetTViewsFor(SimpleComponent);
expect(removeEventListenerSpy).not.toHaveBeenCalled();
const appRef = await hydrate(doc, SimpleComponent, {
hydrationFeatures: [withEventReplay()],
});
appRef.tick();
expect(el.hasAttribute('jsaction')).toBeFalse();
expect((el.firstChild as Element).hasAttribute('jsaction')).toBeFalse();
// Event contract is still listening even if jsaction attributes are removed.
expect(removeEventListenerSpy).not.toHaveBeenCalled();
});

it(`should add 'nonce' attribute to event record script when 'ngCspNonce' is provided`, async () => {
@Component({
standalone: true,
Expand Down

0 comments on commit 95bf0c8

Please sign in to comment.