From 9a7bb906dc2ee97f880d94b755cbf437f0622483 Mon Sep 17 00:00:00 2001 From: Mark berry Date: Sat, 25 Nov 2023 05:32:13 -0600 Subject: [PATCH 1/9] Remove cached NgModules and introduce a global lock during bootstrapping --- .../client/angular-beta/AbstractRenderer.ts | 30 +++++++++++++++++-- .../angular-beta/StorybookWrapperComponent.ts | 28 +++++------------ 2 files changed, 34 insertions(+), 24 deletions(-) diff --git a/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts b/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts index f83fd00dde84..084a1e9fd721 100644 --- a/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts +++ b/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts @@ -3,10 +3,9 @@ import { bootstrapApplication } from '@angular/platform-browser'; import { BehaviorSubject, Subject } from 'rxjs'; import { stringify } from 'telejson'; -import { ICollection, Parameters, StoryFnAngularReturnType } from '../types'; +import { ICollection, StoryFnAngularReturnType } from '../types'; import { getApplication } from './StorybookModule'; import { storyPropsProvider } from './StorybookProvider'; -import { componentNgModules } from './StorybookWrapperComponent'; import { PropertyExtractor } from './utils/PropertyExtractor'; type StoryRenderInfo = { @@ -16,12 +15,13 @@ type StoryRenderInfo = { const applicationRefs = new Map(); +let bootstrappingLock = false; + export abstract class AbstractRenderer { /** * Wait and destroy the platform */ public static resetApplications(domNode?: HTMLElement) { - componentNgModules.clear(); applicationRefs.forEach((appRef, appDOMNode) => { if (!appRef.destroyed && (!domNode || appDOMNode === domNode)) { appRef.destroy(); @@ -129,6 +129,7 @@ export abstract class AbstractRenderer { analyzedMetadata, }); + await this.waitForBootstrappingLock(); const applicationRef = await bootstrapApplication(application, { ...storyFnAngular.applicationConfig, providers: [ @@ -137,12 +138,35 @@ export abstract class AbstractRenderer { ...(storyFnAngular.applicationConfig?.providers ?? []), ], }); + bootstrappingLock = false; applicationRefs.set(targetDOMNode, applicationRef); await this.afterFullRender(); } + /** + * Wait for the previous bootstrapApplication to finish before starting a new one, + * because the compiled component need to be cleared between applications compiling. + * + * Bootstraping multiple applications at the same time will cause the error that + * a component is declared in two modules. + */ + private waitForBootstrappingLock() { + return new Promise((resolve) => { + function checkCondition() { + if (!bootstrappingLock) { + bootstrappingLock = true; + resolve(); + } else { + setTimeout(checkCondition, 100); + } + } + + checkCondition(); + }); + } + /** * Only ASCII alphanumerics can be used as HTML tag name. * https://html.spec.whatwg.org/#elements-2 diff --git a/code/frameworks/angular/src/client/angular-beta/StorybookWrapperComponent.ts b/code/frameworks/angular/src/client/angular-beta/StorybookWrapperComponent.ts index aa5bd589d123..cd39e890166e 100644 --- a/code/frameworks/angular/src/client/angular-beta/StorybookWrapperComponent.ts +++ b/code/frameworks/angular/src/client/angular-beta/StorybookWrapperComponent.ts @@ -31,9 +31,6 @@ const getNonInputsOutputsProps = ( return Object.keys(props).filter((k) => ![...inputs, ...outputs].includes(k)); }; -// component modules cache -export const componentNgModules = new Map>(); - /** * Wraps the story template into a component */ @@ -60,23 +57,12 @@ export const createStorybookWrapperComponent = ({ const { imports, declarations, providers } = analyzedMetadata; - // Only create a new module if it doesn't already exist - // This is to prevent the module from being recreated on every story change - // Declarations & Imports are only added once - // Providers are added on every story change to allow for story-specific providers - let ngModule = componentNgModules.get(storyComponent); - - if (!ngModule) { - @NgModule({ - declarations, - imports, - exports: [...declarations, ...imports], - }) - class StorybookComponentModule {} - - componentNgModules.set(storyComponent, StorybookComponentModule); - ngModule = componentNgModules.get(storyComponent); - } + @NgModule({ + declarations, + imports, + exports: [...declarations, ...imports], + }) + class StorybookComponentModule {} PropertyExtractor.warnImportsModuleWithProviders(analyzedMetadata); @@ -84,7 +70,7 @@ export const createStorybookWrapperComponent = ({ selector, template, standalone: true, - imports: [ngModule], + imports: [StorybookComponentModule], providers, styles, schemas: moduleMetadata.schemas, From 1d42da92fe5f77abc1c3bdf0e0ab5b9879a04b0f Mon Sep 17 00:00:00 2001 From: Mark berry Date: Wed, 29 Nov 2023 19:38:55 -0600 Subject: [PATCH 2/9] Add tests and make bootstrapLock more generic --- .../client/angular-beta/AbstractRenderer.ts | 43 ++----- .../angular-beta/RendererFactory.test.ts | 44 ++++++- .../angular-beta/utils/BootstrapLock.test.ts | 120 ++++++++++++++++++ .../angular-beta/utils/BootstrapLock.ts | 55 ++++++++ 4 files changed, 228 insertions(+), 34 deletions(-) create mode 100644 code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.test.ts create mode 100644 code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.ts diff --git a/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts b/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts index 084a1e9fd721..0b35ccd790e8 100644 --- a/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts +++ b/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts @@ -7,6 +7,7 @@ import { ICollection, StoryFnAngularReturnType } from '../types'; import { getApplication } from './StorybookModule'; import { storyPropsProvider } from './StorybookProvider'; import { PropertyExtractor } from './utils/PropertyExtractor'; +import { bootstrapLock } from './utils/BootstrapLock'; type StoryRenderInfo = { storyFnAngular: StoryFnAngularReturnType; @@ -15,8 +16,6 @@ type StoryRenderInfo = { const applicationRefs = new Map(); -let bootstrappingLock = false; - export abstract class AbstractRenderer { /** * Wait and destroy the platform @@ -129,44 +128,22 @@ export abstract class AbstractRenderer { analyzedMetadata, }); - await this.waitForBootstrappingLock(); - const applicationRef = await bootstrapApplication(application, { - ...storyFnAngular.applicationConfig, - providers: [ - storyPropsProvider(newStoryProps$), - ...analyzedMetadata.applicationProviders, - ...(storyFnAngular.applicationConfig?.providers ?? []), - ], + const applicationRef = await bootstrapLock(targetSelector, () => { + return bootstrapApplication(application, { + ...storyFnAngular.applicationConfig, + providers: [ + storyPropsProvider(newStoryProps$), + ...analyzedMetadata.applicationProviders, + ...(storyFnAngular.applicationConfig?.providers ?? []), + ], + }); }); - bootstrappingLock = false; applicationRefs.set(targetDOMNode, applicationRef); await this.afterFullRender(); } - /** - * Wait for the previous bootstrapApplication to finish before starting a new one, - * because the compiled component need to be cleared between applications compiling. - * - * Bootstraping multiple applications at the same time will cause the error that - * a component is declared in two modules. - */ - private waitForBootstrappingLock() { - return new Promise((resolve) => { - function checkCondition() { - if (!bootstrappingLock) { - bootstrappingLock = true; - resolve(); - } else { - setTimeout(checkCondition, 100); - } - } - - checkCondition(); - }); - } - /** * Only ASCII alphanumerics can be used as HTML tag name. * https://html.spec.whatwg.org/#elements-2 diff --git a/code/frameworks/angular/src/client/angular-beta/RendererFactory.test.ts b/code/frameworks/angular/src/client/angular-beta/RendererFactory.test.ts index 0dc51d15eb6c..bfba51ba173a 100644 --- a/code/frameworks/angular/src/client/angular-beta/RendererFactory.test.ts +++ b/code/frameworks/angular/src/client/angular-beta/RendererFactory.test.ts @@ -17,7 +17,8 @@ describe('RendererFactory', () => { beforeEach(async () => { rendererFactory = new RendererFactory(); document.body.innerHTML = - '
'; + '
' + + '
'; rootTargetDOMNode = global.document.getElementById('storybook-root'); rootDocstargetDOMNode = global.document.getElementById('root-docs'); (platformBrowserDynamic as any).mockImplementation(platformBrowserDynamicTesting); @@ -180,5 +181,46 @@ describe('RendererFactory', () => { const render = await rendererFactory.getRendererInstance(rootDocstargetDOMNode); expect(render).toBeInstanceOf(DocsRenderer); }); + + describe('when bootstrapping multiple stories in parallel', () => { + it('should render both stories', async () => { + @Component({ selector: 'foo', template: '🦊' }) + class FooComponent {} + + const render = await rendererFactory.getRendererInstance( + global.document.getElementById('storybook-docs') + ); + + const targetDOMNode1 = global.document.createElement('div'); + targetDOMNode1.id = 'story-1'; + global.document.getElementById('storybook-docs').appendChild(targetDOMNode1); + + const targetDOMNode2 = global.document.createElement('div'); + targetDOMNode2.id = 'story-2'; + global.document.getElementById('storybook-docs').appendChild(targetDOMNode2); + + await Promise.all([ + render.render({ + storyFnAngular: {}, + forced: false, + component: FooComponent, + targetDOMNode: targetDOMNode1, + }), + render.render({ + storyFnAngular: {}, + forced: false, + component: FooComponent, + targetDOMNode: targetDOMNode2, + }), + ]); + + expect(global.document.querySelector('#story-1 > story-1').innerHTML).toBe( + '🦊' + ); + expect(global.document.querySelector('#story-2 > story-2').innerHTML).toBe( + '🦊' + ); + }); + }); }); }); diff --git a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.test.ts b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.test.ts new file mode 100644 index 000000000000..11fc86750e6d --- /dev/null +++ b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.test.ts @@ -0,0 +1,120 @@ +import { Subject } from 'rxjs'; + +import { bootstrapLock, getCurrentLock } from './BootstrapLock'; + +const tick = (count: number) => new Promise((resolve) => setTimeout(resolve, count)); + +describe('BootstrapLock', () => { + beforeEach(async () => { + jest.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should lock until complete', async () => { + expect(getCurrentLock()).toBeNull(); + + const pendingSubject = new Subject(); + const bootstrapApp = jest.fn().mockImplementation(async () => { + return pendingSubject.toPromise(); + }); + const bootstrapAppFinished = jest.fn(); + + bootstrapLock('story-1', bootstrapApp).then(bootstrapAppFinished); + + await tick(30); + + expect(getCurrentLock()).toBe('story-1'); + expect(bootstrapApp).toHaveBeenCalled(); + expect(bootstrapAppFinished).not.toHaveBeenCalled(); + + pendingSubject.next(); + pendingSubject.complete(); + + await tick(30); + + expect(bootstrapApp).toHaveReturnedTimes(1); + expect(bootstrapAppFinished).toHaveBeenCalled(); + expect(getCurrentLock()).toBeNull(); + }); + + it('should prevent second task, until first task complete', async () => { + expect(getCurrentLock()).toBeNull(); + + const pendingSubject = new Subject(); + const bootstrapApp = jest.fn().mockImplementation(async () => { + return pendingSubject.toPromise(); + }); + const bootstrapAppFinished = jest.fn(); + + const pendingSubject2 = new Subject(); + const bootstrapApp2 = jest.fn().mockImplementation(async () => { + return pendingSubject2.toPromise(); + }); + const bootstrapAppFinished2 = jest.fn(); + + bootstrapLock('story-1', bootstrapApp).then(bootstrapAppFinished); + bootstrapLock('story-2', bootstrapApp2).then(bootstrapAppFinished2); + + await tick(30); + + expect(getCurrentLock()).toBe('story-1'); + expect(bootstrapApp).toHaveBeenCalled(); + expect(bootstrapAppFinished).not.toHaveBeenCalled(); + expect(bootstrapApp2).not.toHaveBeenCalled(); + expect(bootstrapAppFinished2).not.toHaveBeenCalled(); + + pendingSubject.next(); + pendingSubject.complete(); + + await tick(30); + + expect(getCurrentLock()).toBe('story-2'); + expect(bootstrapApp).toHaveReturnedTimes(1); + expect(bootstrapAppFinished).toHaveBeenCalled(); + expect(bootstrapApp2).toHaveBeenCalled(); + expect(bootstrapAppFinished2).not.toHaveBeenCalled(); + + pendingSubject2.next(); + pendingSubject2.complete(); + + await tick(30); + + expect(getCurrentLock()).toBeNull(); + expect(bootstrapApp).toHaveReturnedTimes(1); + expect(bootstrapAppFinished).toHaveBeenCalled(); + expect(bootstrapApp2).toHaveReturnedTimes(1); + expect(bootstrapAppFinished2).toHaveBeenCalled(); + }); + + it('should reset lock on error', async () => { + expect(getCurrentLock()).toBeNull(); + + const pendingSubject = new Subject(); + const bootstrapApp = jest.fn().mockImplementation(async () => { + return pendingSubject.toPromise(); + }); + const bootstrapAppFinished = jest.fn(); + const bootstrapAppError = jest.fn(); + + bootstrapLock('story-1', bootstrapApp).then(bootstrapAppFinished).catch(bootstrapAppError); + + await tick(30); + + expect(getCurrentLock()).toBe('story-1'); + expect(bootstrapApp).toHaveBeenCalled(); + expect(bootstrapAppFinished).not.toHaveBeenCalled(); + + // eslint-disable-next-line local-rules/no-uncategorized-errors + pendingSubject.error(new Error('test error')); + + await tick(30); + + expect(getCurrentLock()).toBeNull(); + expect(bootstrapApp).toHaveReturnedTimes(1); + expect(bootstrapAppFinished).not.toHaveBeenCalled(); + expect(bootstrapAppError).toHaveBeenCalled(); + }); +}); diff --git a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.ts b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.ts new file mode 100644 index 000000000000..77e6a35b4d7c --- /dev/null +++ b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.ts @@ -0,0 +1,55 @@ +import { ApplicationRef } from '@angular/core'; + +let lock: string | null = null; + +/** + * Returns the selector of the component being bootstrapped, or null if no + * component is being bootstrapped. + */ +export const getCurrentLock = (): string | null => { + return lock; +}; + +/** + * Waits for chance to acquire lock for a component to bootstrap. + * + * @param selector the selectory of the component requesting a lock to bootstrap + * @returns + */ +const acquireLock = (selector: string) => { + return new Promise((resolve) => { + function checkLock() { + if (lock === null) { + lock = selector; + resolve(); + } else { + setTimeout(checkLock, 30); + } + } + + checkLock(); + }); +}; + +/** + * Delays bootstrapping until a lock is acquired, so that only one application + * can be bootstrapped at a time. + * + * Bootstrapping multiple applications at once can cause Angular to throw an + * error that a component is declared in multiple modules. + * + * @param selector the selectory of the component requesting a lock to bootstrap + * @param fn callback that should complete the bootstrap process + * @returns ApplicationRef from the completed bootstrap process + */ +export const bootstrapLock = async (selector: string, fn: () => Promise) => { + await acquireLock(selector); + try { + const ref = await fn(); + lock = null; + return ref; + } catch (e) { + lock = null; + throw e; + } +}; From 37e29e497796a2722db772d11bcd7587ce0ddb11 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Fri, 1 Dec 2023 10:05:25 +0100 Subject: [PATCH 3/9] Use a queue instead of loops and timeouts --- .../client/angular-beta/AbstractRenderer.ts | 4 +- .../angular-beta/utils/BootstrapLock.test.ts | 69 +++++++++++-------- .../angular-beta/utils/BootstrapLock.ts | 63 +++++------------ 3 files changed, 59 insertions(+), 77 deletions(-) diff --git a/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts b/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts index 0b35ccd790e8..d3512a53f56c 100644 --- a/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts +++ b/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts @@ -7,7 +7,7 @@ import { ICollection, StoryFnAngularReturnType } from '../types'; import { getApplication } from './StorybookModule'; import { storyPropsProvider } from './StorybookProvider'; import { PropertyExtractor } from './utils/PropertyExtractor'; -import { bootstrapLock } from './utils/BootstrapLock'; +import { queueBootstrapping } from './utils/BootstrapLock'; type StoryRenderInfo = { storyFnAngular: StoryFnAngularReturnType; @@ -128,7 +128,7 @@ export abstract class AbstractRenderer { analyzedMetadata, }); - const applicationRef = await bootstrapLock(targetSelector, () => { + const applicationRef = await queueBootstrapping(() => { return bootstrapApplication(application, { ...storyFnAngular.applicationConfig, providers: [ diff --git a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.test.ts b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.test.ts index 11fc86750e6d..051a92f397ae 100644 --- a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.test.ts +++ b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.test.ts @@ -1,8 +1,8 @@ import { Subject } from 'rxjs'; -import { bootstrapLock, getCurrentLock } from './BootstrapLock'; +import { queueBootstrapping } from './BootstrapLock'; -const tick = (count: number) => new Promise((resolve) => setTimeout(resolve, count)); +const tick = (count = 0) => new Promise((resolve) => setTimeout(resolve, count)); describe('BootstrapLock', () => { beforeEach(async () => { @@ -14,35 +14,28 @@ describe('BootstrapLock', () => { }); it('should lock until complete', async () => { - expect(getCurrentLock()).toBeNull(); - const pendingSubject = new Subject(); const bootstrapApp = jest.fn().mockImplementation(async () => { return pendingSubject.toPromise(); }); const bootstrapAppFinished = jest.fn(); - bootstrapLock('story-1', bootstrapApp).then(bootstrapAppFinished); - - await tick(30); + queueBootstrapping(bootstrapApp).then(() => { + bootstrapAppFinished(); + }); - expect(getCurrentLock()).toBe('story-1'); expect(bootstrapApp).toHaveBeenCalled(); expect(bootstrapAppFinished).not.toHaveBeenCalled(); pendingSubject.next(); pendingSubject.complete(); - await tick(30); + await tick(); - expect(bootstrapApp).toHaveReturnedTimes(1); expect(bootstrapAppFinished).toHaveBeenCalled(); - expect(getCurrentLock()).toBeNull(); }); - it('should prevent second task, until first task complete', async () => { - expect(getCurrentLock()).toBeNull(); - + it('should prevent following tasks, until the preview tasks are complete', async () => { const pendingSubject = new Subject(); const bootstrapApp = jest.fn().mockImplementation(async () => { return pendingSubject.toPromise(); @@ -55,43 +48,63 @@ describe('BootstrapLock', () => { }); const bootstrapAppFinished2 = jest.fn(); - bootstrapLock('story-1', bootstrapApp).then(bootstrapAppFinished); - bootstrapLock('story-2', bootstrapApp2).then(bootstrapAppFinished2); + const pendingSubject3 = new Subject(); + const bootstrapApp3 = jest.fn().mockImplementation(async () => { + return pendingSubject3.toPromise(); + }); + const bootstrapAppFinished3 = jest.fn(); + + queueBootstrapping(bootstrapApp).then(bootstrapAppFinished); + queueBootstrapping(bootstrapApp2).then(bootstrapAppFinished2); + queueBootstrapping(bootstrapApp3).then(bootstrapAppFinished3); - await tick(30); + await tick(); - expect(getCurrentLock()).toBe('story-1'); expect(bootstrapApp).toHaveBeenCalled(); expect(bootstrapAppFinished).not.toHaveBeenCalled(); expect(bootstrapApp2).not.toHaveBeenCalled(); expect(bootstrapAppFinished2).not.toHaveBeenCalled(); + expect(bootstrapApp3).not.toHaveBeenCalled(); + expect(bootstrapAppFinished3).not.toHaveBeenCalled(); pendingSubject.next(); pendingSubject.complete(); - await tick(30); + await tick(); - expect(getCurrentLock()).toBe('story-2'); expect(bootstrapApp).toHaveReturnedTimes(1); expect(bootstrapAppFinished).toHaveBeenCalled(); expect(bootstrapApp2).toHaveBeenCalled(); expect(bootstrapAppFinished2).not.toHaveBeenCalled(); + expect(bootstrapApp3).not.toHaveBeenCalled(); + expect(bootstrapAppFinished3).not.toHaveBeenCalled(); pendingSubject2.next(); pendingSubject2.complete(); - await tick(30); + await tick(); + + expect(bootstrapApp).toHaveReturnedTimes(1); + expect(bootstrapAppFinished).toHaveBeenCalled(); + expect(bootstrapApp2).toHaveReturnedTimes(1); + expect(bootstrapAppFinished2).toHaveBeenCalled(); + expect(bootstrapApp3).toHaveBeenCalled(); + expect(bootstrapAppFinished3).not.toHaveBeenCalled(); + + pendingSubject3.next(); + pendingSubject3.complete(); + + await tick(); - expect(getCurrentLock()).toBeNull(); expect(bootstrapApp).toHaveReturnedTimes(1); expect(bootstrapAppFinished).toHaveBeenCalled(); expect(bootstrapApp2).toHaveReturnedTimes(1); expect(bootstrapAppFinished2).toHaveBeenCalled(); + expect(bootstrapApp3).toHaveReturnedTimes(1); + expect(bootstrapAppFinished3).toHaveBeenCalled(); }); it('should reset lock on error', async () => { - expect(getCurrentLock()).toBeNull(); - const pendingSubject = new Subject(); const bootstrapApp = jest.fn().mockImplementation(async () => { return pendingSubject.toPromise(); @@ -99,20 +112,16 @@ describe('BootstrapLock', () => { const bootstrapAppFinished = jest.fn(); const bootstrapAppError = jest.fn(); - bootstrapLock('story-1', bootstrapApp).then(bootstrapAppFinished).catch(bootstrapAppError); - - await tick(30); + queueBootstrapping(bootstrapApp).then(bootstrapAppFinished).catch(bootstrapAppError); - expect(getCurrentLock()).toBe('story-1'); expect(bootstrapApp).toHaveBeenCalled(); expect(bootstrapAppFinished).not.toHaveBeenCalled(); // eslint-disable-next-line local-rules/no-uncategorized-errors pendingSubject.error(new Error('test error')); - await tick(30); + await tick(); - expect(getCurrentLock()).toBeNull(); expect(bootstrapApp).toHaveReturnedTimes(1); expect(bootstrapAppFinished).not.toHaveBeenCalled(); expect(bootstrapAppError).toHaveBeenCalled(); diff --git a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.ts b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.ts index 77e6a35b4d7c..ddda56986d06 100644 --- a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.ts +++ b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.ts @@ -1,55 +1,28 @@ import { ApplicationRef } from '@angular/core'; -let lock: string | null = null; +const queue: Array<() => Promise> = []; +let isProcessing = false; -/** - * Returns the selector of the component being bootstrapped, or null if no - * component is being bootstrapped. - */ -export const getCurrentLock = (): string | null => { - return lock; -}; +export const queueBootstrapping = (fn: () => Promise): Promise => { + return new Promise((resolve, reject) => { + queue.push(() => fn().then(resolve).catch(reject)); -/** - * Waits for chance to acquire lock for a component to bootstrap. - * - * @param selector the selectory of the component requesting a lock to bootstrap - * @returns - */ -const acquireLock = (selector: string) => { - return new Promise((resolve) => { - function checkLock() { - if (lock === null) { - lock = selector; - resolve(); - } else { - setTimeout(checkLock, 30); - } + if (!isProcessing) { + processQueue(); } - - checkLock(); }); }; -/** - * Delays bootstrapping until a lock is acquired, so that only one application - * can be bootstrapped at a time. - * - * Bootstrapping multiple applications at once can cause Angular to throw an - * error that a component is declared in multiple modules. - * - * @param selector the selectory of the component requesting a lock to bootstrap - * @param fn callback that should complete the bootstrap process - * @returns ApplicationRef from the completed bootstrap process - */ -export const bootstrapLock = async (selector: string, fn: () => Promise) => { - await acquireLock(selector); - try { - const ref = await fn(); - lock = null; - return ref; - } catch (e) { - lock = null; - throw e; +const processQueue = async () => { + isProcessing = true; + + while (queue.length > 0) { + const bootstrappingFn = queue.shift(); + if (bootstrappingFn) { + // eslint-disable-next-line no-await-in-loop + await bootstrappingFn(); + } } + + isProcessing = false; }; From d14ad857b3e4804cf8f3ede5420929ead85e4210 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Fri, 1 Dec 2023 10:43:07 +0100 Subject: [PATCH 4/9] Reset compiled components after bootstrapping --- .../client/angular-beta/AbstractRenderer.ts | 25 ------------------- .../src/client/angular-beta/CanvasRenderer.ts | 4 --- .../src/client/angular-beta/DocsRenderer.ts | 4 --- .../angular-beta/utils/BootstrapLock.ts | 24 +++++++++++++++++- 4 files changed, 23 insertions(+), 34 deletions(-) diff --git a/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts b/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts index d3512a53f56c..645e4653c40e 100644 --- a/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts +++ b/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts @@ -28,27 +28,6 @@ export abstract class AbstractRenderer { }); } - /** - * Reset compiled components because we often want to compile the same component with - * more than one NgModule. - */ - protected static resetCompiledComponents = async () => { - try { - // Clear global Angular component cache in order to be able to re-render the same component across multiple stories - // - // References: - // https://github.com/angular/angular-cli/blob/master/packages/angular_devkit/build_angular/src/webpack/plugins/hmr/hmr-accept.ts#L50 - // https://github.com/angular/angular/blob/2ebe2bcb2fe19bf672316b05f15241fd7fd40803/packages/core/src/render3/jit/module.ts#L377-L384 - const { ɵresetCompiledComponents } = await import('@angular/core'); - ɵresetCompiledComponents(); - } catch (e) { - /** - * noop catch - * This means angular removed or modified ɵresetCompiledComponents - */ - } - }; - protected previousStoryRenderInfo = new Map(); // Observable to change the properties dynamically without reloading angular module&component @@ -68,8 +47,6 @@ export abstract class AbstractRenderer { protected abstract beforeFullRender(domNode?: HTMLElement): Promise; - protected abstract afterFullRender(): Promise; - /** * Bootstrap main angular module with main component or send only new `props` with storyProps$ * @@ -140,8 +117,6 @@ export abstract class AbstractRenderer { }); applicationRefs.set(targetDOMNode, applicationRef); - - await this.afterFullRender(); } /** diff --git a/code/frameworks/angular/src/client/angular-beta/CanvasRenderer.ts b/code/frameworks/angular/src/client/angular-beta/CanvasRenderer.ts index 994733657f78..eeb6812ddab3 100644 --- a/code/frameworks/angular/src/client/angular-beta/CanvasRenderer.ts +++ b/code/frameworks/angular/src/client/angular-beta/CanvasRenderer.ts @@ -15,8 +15,4 @@ export class CanvasRenderer extends AbstractRenderer { async beforeFullRender(): Promise { CanvasRenderer.resetApplications(); } - - async afterFullRender(): Promise { - await AbstractRenderer.resetCompiledComponents(); - } } diff --git a/code/frameworks/angular/src/client/angular-beta/DocsRenderer.ts b/code/frameworks/angular/src/client/angular-beta/DocsRenderer.ts index d51573376fcb..4bffa45c4c5d 100644 --- a/code/frameworks/angular/src/client/angular-beta/DocsRenderer.ts +++ b/code/frameworks/angular/src/client/angular-beta/DocsRenderer.ts @@ -41,8 +41,4 @@ export class DocsRenderer extends AbstractRenderer { async beforeFullRender(domNode?: HTMLElement): Promise { DocsRenderer.resetApplications(domNode); } - - async afterFullRender(): Promise { - await AbstractRenderer.resetCompiledComponents(); - } } diff --git a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.ts b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.ts index ddda56986d06..a976f2738ec2 100644 --- a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.ts +++ b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.ts @@ -1,8 +1,30 @@ +/* eslint-disable no-await-in-loop */ import { ApplicationRef } from '@angular/core'; const queue: Array<() => Promise> = []; let isProcessing = false; +/** + * Reset compiled components because we often want to compile the same component with + * more than one NgModule. + */ +const resetCompiledComponents = async () => { + try { + // Clear global Angular component cache in order to be able to re-render the same component across multiple stories + // + // References: + // https://github.com/angular/angular-cli/blob/master/packages/angular_devkit/build_angular/src/webpack/plugins/hmr/hmr-accept.ts#L50 + // https://github.com/angular/angular/blob/2ebe2bcb2fe19bf672316b05f15241fd7fd40803/packages/core/src/render3/jit/module.ts#L377-L384 + const { ɵresetCompiledComponents } = await import('@angular/core'); + ɵresetCompiledComponents(); + } catch (e) { + /** + * noop catch + * This means angular removed or modified ɵresetCompiledComponents + */ + } +}; + export const queueBootstrapping = (fn: () => Promise): Promise => { return new Promise((resolve, reject) => { queue.push(() => fn().then(resolve).catch(reject)); @@ -19,8 +41,8 @@ const processQueue = async () => { while (queue.length > 0) { const bootstrappingFn = queue.shift(); if (bootstrappingFn) { - // eslint-disable-next-line no-await-in-loop await bootstrappingFn(); + await resetCompiledComponents(); } } From f729eb8f236ec9a81d9c625ba445edbbaeee056e Mon Sep 17 00:00:00 2001 From: Mark berry Date: Fri, 1 Dec 2023 23:23:39 -0600 Subject: [PATCH 5/9] Update test and names to refer to "queue" instead of "lock" --- .../client/angular-beta/AbstractRenderer.ts | 2 +- ...rapLock.test.ts => BootstrapQueue.test.ts} | 51 ++++++++++++++----- .../{BootstrapLock.ts => BootstrapQueue.ts} | 12 +++++ 3 files changed, 51 insertions(+), 14 deletions(-) rename code/frameworks/angular/src/client/angular-beta/utils/{BootstrapLock.test.ts => BootstrapQueue.test.ts} (68%) rename code/frameworks/angular/src/client/angular-beta/utils/{BootstrapLock.ts => BootstrapQueue.ts} (76%) diff --git a/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts b/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts index 645e4653c40e..9e3f363efbac 100644 --- a/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts +++ b/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts @@ -7,7 +7,7 @@ import { ICollection, StoryFnAngularReturnType } from '../types'; import { getApplication } from './StorybookModule'; import { storyPropsProvider } from './StorybookProvider'; import { PropertyExtractor } from './utils/PropertyExtractor'; -import { queueBootstrapping } from './utils/BootstrapLock'; +import { queueBootstrapping } from './utils/BootstrapQueue'; type StoryRenderInfo = { storyFnAngular: StoryFnAngularReturnType; diff --git a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.test.ts b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.test.ts similarity index 68% rename from code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.test.ts rename to code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.test.ts index 051a92f397ae..a7625e37b6d8 100644 --- a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.test.ts +++ b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.test.ts @@ -1,10 +1,10 @@ -import { Subject } from 'rxjs'; +import { Subject, lastValueFrom } from 'rxjs'; -import { queueBootstrapping } from './BootstrapLock'; +import { queueBootstrapping } from './BootstrapQueue'; const tick = (count = 0) => new Promise((resolve) => setTimeout(resolve, count)); -describe('BootstrapLock', () => { +describe('BootstrapQueue', () => { beforeEach(async () => { jest.spyOn(console, 'log').mockImplementation(() => {}); }); @@ -13,10 +13,10 @@ describe('BootstrapLock', () => { jest.clearAllMocks(); }); - it('should lock until complete', async () => { + it('should wait until complete', async () => { const pendingSubject = new Subject(); const bootstrapApp = jest.fn().mockImplementation(async () => { - return pendingSubject.toPromise(); + return lastValueFrom(pendingSubject); }); const bootstrapAppFinished = jest.fn(); @@ -38,19 +38,19 @@ describe('BootstrapLock', () => { it('should prevent following tasks, until the preview tasks are complete', async () => { const pendingSubject = new Subject(); const bootstrapApp = jest.fn().mockImplementation(async () => { - return pendingSubject.toPromise(); + return lastValueFrom(pendingSubject); }); const bootstrapAppFinished = jest.fn(); const pendingSubject2 = new Subject(); const bootstrapApp2 = jest.fn().mockImplementation(async () => { - return pendingSubject2.toPromise(); + return lastValueFrom(pendingSubject2); }); const bootstrapAppFinished2 = jest.fn(); const pendingSubject3 = new Subject(); const bootstrapApp3 = jest.fn().mockImplementation(async () => { - return pendingSubject3.toPromise(); + return lastValueFrom(pendingSubject3); }); const bootstrapAppFinished3 = jest.fn(); @@ -104,26 +104,51 @@ describe('BootstrapLock', () => { expect(bootstrapAppFinished3).toHaveBeenCalled(); }); - it('should reset lock on error', async () => { + it('should throw and continue next bootstrap on error', async () => { const pendingSubject = new Subject(); const bootstrapApp = jest.fn().mockImplementation(async () => { - return pendingSubject.toPromise(); + return lastValueFrom(pendingSubject); }); const bootstrapAppFinished = jest.fn(); const bootstrapAppError = jest.fn(); + const pendingSubject2 = new Subject(); + const bootstrapApp2 = jest.fn().mockImplementation(async () => { + return lastValueFrom(pendingSubject2); + }); + const bootstrapAppFinished2 = jest.fn(); + const bootstrapAppError2 = jest.fn(); + queueBootstrapping(bootstrapApp).then(bootstrapAppFinished).catch(bootstrapAppError); + queueBootstrapping(bootstrapApp2).then(bootstrapAppFinished2).catch(bootstrapAppError2); - expect(bootstrapApp).toHaveBeenCalled(); + expect(bootstrapApp).toHaveBeenCalledTimes(1); expect(bootstrapAppFinished).not.toHaveBeenCalled(); + expect(bootstrapApp2).not.toHaveBeenCalled(); + expect(bootstrapAppFinished2).not.toHaveBeenCalled(); // eslint-disable-next-line local-rules/no-uncategorized-errors pendingSubject.error(new Error('test error')); await tick(); - expect(bootstrapApp).toHaveReturnedTimes(1); + expect(bootstrapApp).toHaveBeenCalledTimes(1); + expect(bootstrapAppFinished).not.toHaveBeenCalled(); + expect(bootstrapAppError).toHaveBeenCalledTimes(1); + expect(bootstrapApp2).toHaveBeenCalledTimes(1); + expect(bootstrapAppFinished2).not.toHaveBeenCalled(); + expect(bootstrapAppError2).not.toHaveBeenCalled(); + + pendingSubject2.next(); + pendingSubject2.complete(); + + await tick(); + + expect(bootstrapApp).toHaveBeenCalledTimes(1); expect(bootstrapAppFinished).not.toHaveBeenCalled(); - expect(bootstrapAppError).toHaveBeenCalled(); + expect(bootstrapAppError).toHaveBeenCalledTimes(1); + expect(bootstrapApp2).toHaveBeenCalledTimes(1); + expect(bootstrapAppFinished2).toHaveBeenCalledTimes(1); + expect(bootstrapAppError2).not.toHaveBeenCalled(); }); }); diff --git a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.ts b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts similarity index 76% rename from code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.ts rename to code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts index a976f2738ec2..a5c7ca6df8ce 100644 --- a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapLock.ts +++ b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts @@ -25,6 +25,18 @@ const resetCompiledComponents = async () => { } }; +/** + * Queue bootstrapping, so that only one application can be bootstrapped at a + * time. + * + * Bootstrapping multiple applications at once can cause Angular to throw an + * error that a component is declared in multiple modules. This avoids two + * stories confusing the Angular compiler, by bootstrapping more that one + * application at a time. + * + * @param fn callback that should complete the bootstrap process + * @returns ApplicationRef from the completed bootstrap process + */ export const queueBootstrapping = (fn: () => Promise): Promise => { return new Promise((resolve, reject) => { queue.push(() => fn().then(resolve).catch(reject)); From 29ab19245a6f69aeef356ac5e368d89ecb589daa Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Mon, 4 Dec 2023 09:32:01 +0100 Subject: [PATCH 6/9] Reset compiled components before bootstraping --- .../src/client/angular-beta/utils/BootstrapQueue.test.ts | 4 ++++ .../angular/src/client/angular-beta/utils/BootstrapQueue.ts | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.test.ts b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.test.ts index a7625e37b6d8..04dcdd65e077 100644 --- a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.test.ts +++ b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.test.ts @@ -24,6 +24,8 @@ describe('BootstrapQueue', () => { bootstrapAppFinished(); }); + await tick(); + expect(bootstrapApp).toHaveBeenCalled(); expect(bootstrapAppFinished).not.toHaveBeenCalled(); @@ -122,6 +124,8 @@ describe('BootstrapQueue', () => { queueBootstrapping(bootstrapApp).then(bootstrapAppFinished).catch(bootstrapAppError); queueBootstrapping(bootstrapApp2).then(bootstrapAppFinished2).catch(bootstrapAppError2); + await tick(); + expect(bootstrapApp).toHaveBeenCalledTimes(1); expect(bootstrapAppFinished).not.toHaveBeenCalled(); expect(bootstrapApp2).not.toHaveBeenCalled(); diff --git a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts index a5c7ca6df8ce..e708453223d7 100644 --- a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts +++ b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts @@ -53,8 +53,8 @@ const processQueue = async () => { while (queue.length > 0) { const bootstrappingFn = queue.shift(); if (bootstrappingFn) { - await bootstrappingFn(); await resetCompiledComponents(); + await bootstrappingFn(); } } From f71ca84cc9d783c688443c84e6d60020e9a6b5f8 Mon Sep 17 00:00:00 2001 From: Mark Berry Date: Thu, 28 Dec 2023 17:13:42 -0600 Subject: [PATCH 7/9] Migrate tests to vitest --- .../angular-beta/utils/BootstrapQueue.test.ts | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.test.ts b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.test.ts index 04dcdd65e077..542f5e3b6df7 100644 --- a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.test.ts +++ b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.test.ts @@ -1,4 +1,5 @@ import { Subject, lastValueFrom } from 'rxjs'; +import { vi, describe, it, expect, beforeEach } from 'vitest'; import { queueBootstrapping } from './BootstrapQueue'; @@ -6,19 +7,19 @@ const tick = (count = 0) => new Promise((resolve) => setTimeout(resolve, count)) describe('BootstrapQueue', () => { beforeEach(async () => { - jest.spyOn(console, 'log').mockImplementation(() => {}); + vi.spyOn(console, 'log').mockImplementation(() => {}); }); afterEach(() => { - jest.clearAllMocks(); + vi.clearAllMocks(); }); it('should wait until complete', async () => { const pendingSubject = new Subject(); - const bootstrapApp = jest.fn().mockImplementation(async () => { + const bootstrapApp = vi.fn().mockImplementation(async () => { return lastValueFrom(pendingSubject); }); - const bootstrapAppFinished = jest.fn(); + const bootstrapAppFinished = vi.fn(); queueBootstrapping(bootstrapApp).then(() => { bootstrapAppFinished(); @@ -39,22 +40,22 @@ describe('BootstrapQueue', () => { it('should prevent following tasks, until the preview tasks are complete', async () => { const pendingSubject = new Subject(); - const bootstrapApp = jest.fn().mockImplementation(async () => { + const bootstrapApp = vi.fn().mockImplementation(async () => { return lastValueFrom(pendingSubject); }); - const bootstrapAppFinished = jest.fn(); + const bootstrapAppFinished = vi.fn(); const pendingSubject2 = new Subject(); - const bootstrapApp2 = jest.fn().mockImplementation(async () => { + const bootstrapApp2 = vi.fn().mockImplementation(async () => { return lastValueFrom(pendingSubject2); }); - const bootstrapAppFinished2 = jest.fn(); + const bootstrapAppFinished2 = vi.fn(); const pendingSubject3 = new Subject(); - const bootstrapApp3 = jest.fn().mockImplementation(async () => { + const bootstrapApp3 = vi.fn().mockImplementation(async () => { return lastValueFrom(pendingSubject3); }); - const bootstrapAppFinished3 = jest.fn(); + const bootstrapAppFinished3 = vi.fn(); queueBootstrapping(bootstrapApp).then(bootstrapAppFinished); queueBootstrapping(bootstrapApp2).then(bootstrapAppFinished2); @@ -108,18 +109,18 @@ describe('BootstrapQueue', () => { it('should throw and continue next bootstrap on error', async () => { const pendingSubject = new Subject(); - const bootstrapApp = jest.fn().mockImplementation(async () => { + const bootstrapApp = vi.fn().mockImplementation(async () => { return lastValueFrom(pendingSubject); }); - const bootstrapAppFinished = jest.fn(); - const bootstrapAppError = jest.fn(); + const bootstrapAppFinished = vi.fn(); + const bootstrapAppError = vi.fn(); const pendingSubject2 = new Subject(); - const bootstrapApp2 = jest.fn().mockImplementation(async () => { + const bootstrapApp2 = vi.fn().mockImplementation(async () => { return lastValueFrom(pendingSubject2); }); - const bootstrapAppFinished2 = jest.fn(); - const bootstrapAppError2 = jest.fn(); + const bootstrapAppFinished2 = vi.fn(); + const bootstrapAppError2 = vi.fn(); queueBootstrapping(bootstrapApp).then(bootstrapAppFinished).catch(bootstrapAppError); queueBootstrapping(bootstrapApp2).then(bootstrapAppFinished2).catch(bootstrapAppError2); @@ -131,7 +132,6 @@ describe('BootstrapQueue', () => { expect(bootstrapApp2).not.toHaveBeenCalled(); expect(bootstrapAppFinished2).not.toHaveBeenCalled(); - // eslint-disable-next-line local-rules/no-uncategorized-errors pendingSubject.error(new Error('test error')); await tick(); From e94ad415052a9d8882daabeda87ab71a9b6713fa Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Wed, 17 Jan 2024 09:50:06 +0100 Subject: [PATCH 8/9] Fix flaky Angular tests --- .../angular-beta/utils/BootstrapQueue.test.ts | 54 ++++++++++++++----- .../angular-beta/utils/BootstrapQueue.ts | 1 - 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.test.ts b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.test.ts index 542f5e3b6df7..f518e7daf6e9 100644 --- a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.test.ts +++ b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.test.ts @@ -1,10 +1,9 @@ import { Subject, lastValueFrom } from 'rxjs'; -import { vi, describe, it, expect, beforeEach } from 'vitest'; +import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; +import assert from 'node:assert'; import { queueBootstrapping } from './BootstrapQueue'; -const tick = (count = 0) => new Promise((resolve) => setTimeout(resolve, count)); - describe('BootstrapQueue', () => { beforeEach(async () => { vi.spyOn(console, 'log').mockImplementation(() => {}); @@ -25,7 +24,9 @@ describe('BootstrapQueue', () => { bootstrapAppFinished(); }); - await tick(); + await vi.waitFor(() => { + assert(bootstrapApp.mock.calls.length === 1, 'bootstrapApp should have been called once'); + }); expect(bootstrapApp).toHaveBeenCalled(); expect(bootstrapAppFinished).not.toHaveBeenCalled(); @@ -33,7 +34,12 @@ describe('BootstrapQueue', () => { pendingSubject.next(); pendingSubject.complete(); - await tick(); + await vi.waitFor(() => { + assert( + bootstrapAppFinished.mock.calls.length === 1, + 'bootstrapApp should have been called once' + ); + }); expect(bootstrapAppFinished).toHaveBeenCalled(); }); @@ -61,7 +67,9 @@ describe('BootstrapQueue', () => { queueBootstrapping(bootstrapApp2).then(bootstrapAppFinished2); queueBootstrapping(bootstrapApp3).then(bootstrapAppFinished3); - await tick(); + await vi.waitFor(() => { + assert(bootstrapApp.mock.calls.length === 1, 'bootstrapApp should have been called once'); + }); expect(bootstrapApp).toHaveBeenCalled(); expect(bootstrapAppFinished).not.toHaveBeenCalled(); @@ -73,7 +81,9 @@ describe('BootstrapQueue', () => { pendingSubject.next(); pendingSubject.complete(); - await tick(); + await vi.waitFor(() => { + assert(bootstrapApp2.mock.calls.length === 1, 'bootstrapApp2 should have been called once'); + }); expect(bootstrapApp).toHaveReturnedTimes(1); expect(bootstrapAppFinished).toHaveBeenCalled(); @@ -85,7 +95,9 @@ describe('BootstrapQueue', () => { pendingSubject2.next(); pendingSubject2.complete(); - await tick(); + await vi.waitFor(() => { + assert(bootstrapApp3.mock.calls.length === 1, 'bootstrapApp3 should have been called once'); + }); expect(bootstrapApp).toHaveReturnedTimes(1); expect(bootstrapAppFinished).toHaveBeenCalled(); @@ -97,7 +109,12 @@ describe('BootstrapQueue', () => { pendingSubject3.next(); pendingSubject3.complete(); - await tick(); + await vi.waitFor(() => { + assert( + bootstrapAppFinished3.mock.calls.length === 1, + 'bootstrapAppFinished3 should have been called once' + ); + }); expect(bootstrapApp).toHaveReturnedTimes(1); expect(bootstrapAppFinished).toHaveBeenCalled(); @@ -125,16 +142,22 @@ describe('BootstrapQueue', () => { queueBootstrapping(bootstrapApp).then(bootstrapAppFinished).catch(bootstrapAppError); queueBootstrapping(bootstrapApp2).then(bootstrapAppFinished2).catch(bootstrapAppError2); - await tick(); + await vi.waitFor(() => { + assert(bootstrapApp.mock.calls.length === 1, 'bootstrapApp should have been called once'); + }); expect(bootstrapApp).toHaveBeenCalledTimes(1); expect(bootstrapAppFinished).not.toHaveBeenCalled(); expect(bootstrapApp2).not.toHaveBeenCalled(); - expect(bootstrapAppFinished2).not.toHaveBeenCalled(); pendingSubject.error(new Error('test error')); - await tick(); + await vi.waitFor(() => { + assert( + bootstrapAppError.mock.calls.length === 1, + 'bootstrapAppError should have been called once' + ); + }); expect(bootstrapApp).toHaveBeenCalledTimes(1); expect(bootstrapAppFinished).not.toHaveBeenCalled(); @@ -146,7 +169,12 @@ describe('BootstrapQueue', () => { pendingSubject2.next(); pendingSubject2.complete(); - await tick(); + await vi.waitFor(() => { + assert( + bootstrapAppFinished2.mock.calls.length === 1, + 'bootstrapAppFinished2 should have been called once' + ); + }); expect(bootstrapApp).toHaveBeenCalledTimes(1); expect(bootstrapAppFinished).not.toHaveBeenCalled(); diff --git a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts index e708453223d7..b1b52baa85a8 100644 --- a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts +++ b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts @@ -1,4 +1,3 @@ -/* eslint-disable no-await-in-loop */ import { ApplicationRef } from '@angular/core'; const queue: Array<() => Promise> = []; From 793cea6ffef0240a2c95d16b7a17e8d1f574c612 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Wed, 17 Jan 2024 13:28:01 +0100 Subject: [PATCH 9/9] Refactor BootstrapQueue.ts to improve code execution order --- .../angular/src/client/angular-beta/utils/BootstrapQueue.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts index b1b52baa85a8..20b81c860413 100644 --- a/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts +++ b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts @@ -52,8 +52,8 @@ const processQueue = async () => { while (queue.length > 0) { const bootstrappingFn = queue.shift(); if (bootstrappingFn) { - await resetCompiledComponents(); await bootstrappingFn(); + await resetCompiledComponents(); } }