From ec56161c4916e79bd07f31bb8b89072d396f7667 Mon Sep 17 00:00:00 2001 From: arturovt Date: Mon, 20 Jan 2025 21:04:47 +0200 Subject: [PATCH] fix(core): prevent stash listener conflicts The stash event listener is a global function that might be unsafely overridden if multiple microfrontend applications exist on the page. In this commit, we create a map of `APP_ID` to stash event listener functions. This map prevents conflicts because multiple applications might be bootstrapped simultaneously on the client (one rendered on the server and one rendering only on the client). I.e., the code that might be used is: ```ts // Given that `app-root` is rendered on the server bootstrapApplication(AppComponent, appConfig); bootstrapApplication(BlogRootComponent, appBlogConfig); ``` Two bootstrapped applications would conflict and override each other's code. --- packages/core/src/hydration/event_replay.ts | 9 +-- packages/core/src/render3/view/listeners.ts | 19 +++-- .../forms_reactive/bundle.golden_symbols.json | 1 + .../bundle.golden_symbols.json | 1 + .../router/bundle.golden_symbols.json | 1 + .../platform-server/test/event_replay_spec.ts | 71 ++++++++++++++++++- 6 files changed, 92 insertions(+), 10 deletions(-) diff --git a/packages/core/src/hydration/event_replay.ts b/packages/core/src/hydration/event_replay.ts index 4d58a97d619e..f9018f0a4dfd 100644 --- a/packages/core/src/hydration/event_replay.ts +++ b/packages/core/src/hydration/event_replay.ts @@ -44,7 +44,7 @@ import {APP_ID} from '../application/application_tokens'; import {performanceMarkFeature} from '../util/performance'; import {triggerHydrationFromBlockName} from '../defer/triggering'; import {isIncrementalHydrationEnabled} from './utils'; -import {setStashFn} from '../render3/view/listeners'; +import {clearStashFn, setStashFn} from '../render3/view/listeners'; /** Apps in which we've enabled event replay. * This is to prevent initializing event replay more than once per app. @@ -106,7 +106,8 @@ export function withEventReplay(): Provider[] { if (!appsWithEventReplay.has(appRef)) { const jsActionMap = inject(JSACTION_BLOCK_ELEMENT_MAP); if (shouldEnableEventReplay(injector)) { - setStashFn((rEl: RNode, eventName: string, listenerFn: VoidFunction) => { + const appId = injector.get(APP_ID); + setStashFn(appId, (rEl: RNode, eventName: string, listenerFn: VoidFunction) => { // If a user binds to a ng-container and uses a directive that binds using a host listener, // this element could be a comment node. So we need to ensure we have an actual element // node before stashing anything. @@ -122,7 +123,6 @@ export function withEventReplay(): Provider[] { { provide: APP_BOOTSTRAP_LISTENER, useFactory: () => { - const appId = inject(APP_ID); const appRef = inject(ApplicationRef); const {injector} = appRef; @@ -140,6 +140,7 @@ export function withEventReplay(): Provider[] { appsWithEventReplay.delete(appRef); // Ensure that we're always safe calling this in the browser. if (typeof ngServerMode !== 'undefined' && !ngServerMode) { + const appId = injector.get(APP_ID); // `_ejsa` should be deleted when the app is destroyed, ensuring that // no elements are still captured in the global list and are not prevented // from being garbage collected. @@ -147,7 +148,7 @@ export function withEventReplay(): Provider[] { // Clean up the reference to the function set by the environment initializer, // as the function closure may capture injected elements and prevent them // from being properly garbage collected. - setStashFn(() => {}); + clearStashFn(appId); } }); diff --git a/packages/core/src/render3/view/listeners.ts b/packages/core/src/render3/view/listeners.ts index 8a322a212e68..d3fa1daca0e3 100644 --- a/packages/core/src/render3/view/listeners.ts +++ b/packages/core/src/render3/view/listeners.ts @@ -26,6 +26,7 @@ import {RElement, RNode} from '../interfaces/renderer_dom'; import {GlobalTargetResolver, Renderer} from '../interfaces/renderer'; import {assertNotSame} from '../../util/assert'; import {handleUncaughtError} from '../instructions/shared'; +import {APP_ID} from '../../application/application_tokens'; /** Shorthand for an event listener callback function to reduce duplication. */ export type EventCallback = (event?: any) => any; @@ -34,15 +35,21 @@ export type EventCallback = (event?: any) => any; export type WrappedEventCallback = EventCallback & {__wrapped: boolean}; /** - * Contains a reference to a function that disables event replay feature + * Represents a signature of 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 stashEventListener = (el: RNode, eventName: string, listenerFn: EventCallback) => {}; +type StashEventListener = (el: RNode, eventName: string, listenerFn: EventCallback) => void; -export function setStashFn(fn: typeof stashEventListener) { - stashEventListener = fn; +const stashEventListeners = new Map(); + +export function setStashFn(appId: string, fn: StashEventListener) { + stashEventListeners.set(appId, fn); +} + +export function clearStashFn(appId: string) { + stashEventListeners.delete(appId); } /** @@ -172,7 +179,9 @@ export function listenToDomEvent( } else { const native = getNativeByTNode(tNode, lView) as RElement; const target = eventTargetResolver ? eventTargetResolver(native) : native; - stashEventListener(target as RElement, eventName, wrappedListener); + const appId = lView[INJECTOR].get(APP_ID); + const stashEventListener = stashEventListeners.get(appId); + stashEventListener?.(target as RElement, eventName, wrappedListener); const cleanupFn = renderer.listen(target as RElement, eventName, wrappedListener); const idxOrTargetGetter = eventTargetResolver diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json index f1d8e7986216..b002f5fee727 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -611,6 +611,7 @@ "signal", "signalAsReadonlyFn", "signalSetFn", + "stashEventListeners", "storeLViewOnDestroy", "storeListenerCleanup", "stringify", diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json index 5026066624b1..88b14ef95e4c 100644 --- a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json @@ -606,6 +606,7 @@ "signal", "signalAsReadonlyFn", "signalSetFn", + "stashEventListeners", "storeLViewOnDestroy", "storeListenerCleanup", "stringify", diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index 36916ae8721d..bc84bd940abe 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -692,6 +692,7 @@ "split", "squashSegmentGroup", "standardizeConfig", + "stashEventListeners", "storeLViewOnDestroy", "storeListenerCleanup", "stringify", diff --git a/packages/platform-server/test/event_replay_spec.ts b/packages/platform-server/test/event_replay_spec.ts index da398af981ee..3442adec4919 100644 --- a/packages/platform-server/test/event_replay_spec.ts +++ b/packages/platform-server/test/event_replay_spec.ts @@ -18,7 +18,11 @@ import { PLATFORM_ID, } from '@angular/core'; import {isPlatformBrowser} from '@angular/common'; -import {withEventReplay} from '@angular/platform-browser'; +import { + bootstrapApplication, + provideClientHydration, + withEventReplay, +} from '@angular/platform-browser'; import {EventPhase} from '@angular/core/primitives/event-dispatch'; @@ -111,6 +115,71 @@ describe('event replay', () => { expect(onClickSpy).toHaveBeenCalled(); }); + it('stash event listeners should not conflict when multiple apps are bootstrapped', async () => { + const onClickSpy = jasmine.createSpy(); + + @Component({ + selector: 'app', + standalone: true, + template: ` + + `, + }) + class AppComponent_1 { + onClick = onClickSpy; + } + + @Component({ + selector: 'app-2', + standalone: true, + template: ` + + `, + }) + class AppComponent_2 { + onClick() {} + } + + const hydrationFeatures = () => [withEventReplay()]; + const docHtml = ` + + + + ${EVENT_DISPATCH_SCRIPT} + + + + + `; + const html = await ssr(AppComponent_1, {hydrationFeatures, doc: docHtml}); + const ssrContents = getAppContents(html); + const doc = getDocument(); + + prepareEnvironment(doc, ssrContents); + resetTViewsFor(AppComponent_1); + + const btn = doc.getElementById('btn-1')!; + btn.click(); + + // It's hard to server-side render multiple applications in this + // particular unit test and hydrate them on the client, so instead, + // let's render the application with `provideClientHydration` to enable + // event replay features and ensure the stash event listener is set. + await bootstrapApplication(AppComponent_2, { + providers: [ + provideClientHydration(withEventReplay()), + {provide: APP_ID, useValue: 'random_name'}, + ], + }); + + // Now let's hydrate the second application and ensure that the + // button click event has been replayed. + const appRef = await hydrate(doc, AppComponent_1, {hydrationFeatures}); + appRef.tick(); + + expect(onClickSpy).toHaveBeenCalled(); + }); + it('should cleanup `window._ejsas[appId]` once app is destroyed', async () => { @Component({ selector: 'app',