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
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.
  • Loading branch information
iteriani committed Apr 25, 2024
1 parent bf8814c commit 8840c70
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 2 deletions.
17 changes: 15 additions & 2 deletions packages/core/src/hydration/event_replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ import {Dispatcher, EventContract, EventInfoWrapper, registerDispatcher} from '@

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 {setReplaceJsactionCode} 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 @@ -39,6 +40,17 @@ export function withEventReplay(): Provider[] {
provide: IS_EVENT_REPLAY_ENABLED,
useValue: true,
},
{
provide: ENVIRONMENT_INITIALIZER,
useValue: () => {
setReplaceJsactionCode((el: RElement) => {
if (el.hasAttribute('jsaction')) {
el.removeAttribute('jsaction');
}
});
},
multi: true,
},
{
provide: APP_BOOTSTRAP_LISTENER,
useFactory: () => {
Expand Down Expand Up @@ -173,6 +185,7 @@ function handleEvent(event: EventInfoWrapper) {
listener(origEvent);
}
}
nativeElement.removeAttribute('jsaction');
}
}

Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/render3/instructions/listener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ import {getComponentLViewByIndex, getNativeByTNode, unwrapRNode} from '../util/v
import {markViewDirty} from './mark_view_dirty';
import {getOrCreateLViewCleanup, getOrCreateTViewCleanup, handleError, loadComponentRenderer} from './shared';

let replaceJsaction = (el: RElement) => {};

export function setReplaceJsactionCode(fn: (el: RElement) => void) {
replaceJsaction = fn;
}

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

replaceJsaction(native);

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1592,6 +1592,9 @@
{
"name": "renderView"
},
{
"name": "replaceJsaction"
},
{
"name": "reportUnhandledError"
},
Expand Down
43 changes: 43 additions & 0 deletions packages/platform-server/test/event_replay_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,49 @@ describe('event replay', () => {
expect(clickSpy).toHaveBeenCalled();
});

it('should remove jsaction attributes and stop listening after hydration.', 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 originalRemoveEventListener = doc.body.removeEventListener;
const removeEventListenerSpy = jasmine.createSpy();
doc.body.removeEventListener = function (
type: string,
listener: EventListenerOrEventListenerObject,
options: boolean | EventListenerOptions | undefined,
) {
originalRemoveEventListener.call(doc.body, type, listener, options);
removeEventListenerSpy();
};
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 8840c70

Please sign in to comment.