diff --git a/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts b/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts index 4d152a7afcfc..61823a60e7f9 100644 --- a/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts +++ b/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts @@ -7,8 +7,8 @@ import { stringify } from 'telejson'; import { ICollection, StoryFnAngularReturnType } from '../types'; import { getApplication } from './StorybookModule'; import { storyPropsProvider } from './StorybookProvider'; -import { componentNgModules } from './StorybookWrapperComponent'; import { PropertyExtractor } from './utils/PropertyExtractor'; +import { queueBootstrapping } from './utils/BootstrapQueue'; type StoryRenderInfo = { storyFnAngular: StoryFnAngularReturnType; @@ -30,7 +30,6 @@ 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(); @@ -38,27 +37,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 @@ -77,8 +55,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$ * @@ -144,18 +120,18 @@ export abstract class AbstractRenderer { analyzedMetadata, }); - const applicationRef = await bootstrapApplication(application, { - ...storyFnAngular.applicationConfig, - providers: [ - storyPropsProvider(newStoryProps$), - ...analyzedMetadata.applicationProviders, - ...(storyFnAngular.applicationConfig?.providers ?? []), - ], + const applicationRef = await queueBootstrapping(() => { + return bootstrapApplication(application, { + ...storyFnAngular.applicationConfig, + providers: [ + storyPropsProvider(newStoryProps$), + ...analyzedMetadata.applicationProviders, + ...(storyFnAngular.applicationConfig?.providers ?? []), + ], + }); }); 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 9b7629854626..788d6283cc1e 100644 --- a/code/frameworks/angular/src/client/angular-beta/DocsRenderer.ts +++ b/code/frameworks/angular/src/client/angular-beta/DocsRenderer.ts @@ -44,10 +44,6 @@ export class DocsRenderer extends AbstractRenderer { DocsRenderer.resetApplications(domNode); } - async afterFullRender(): Promise { - await AbstractRenderer.resetCompiledComponents(); - } - protected override initAngularRootElement( targetDOMNode: HTMLElement, targetSelector: string 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 fe81eddb1a33..bd87875ed4d7 100644 --- a/code/frameworks/angular/src/client/angular-beta/RendererFactory.test.ts +++ b/code/frameworks/angular/src/client/angular-beta/RendererFactory.test.ts @@ -224,5 +224,46 @@ describe('RendererFactory', () => { ); }); }); + + 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/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, 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 new file mode 100644 index 000000000000..f518e7daf6e9 --- /dev/null +++ b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.test.ts @@ -0,0 +1,186 @@ +import { Subject, lastValueFrom } from 'rxjs'; +import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; +import assert from 'node:assert'; + +import { queueBootstrapping } from './BootstrapQueue'; + +describe('BootstrapQueue', () => { + beforeEach(async () => { + vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('should wait until complete', async () => { + const pendingSubject = new Subject(); + const bootstrapApp = vi.fn().mockImplementation(async () => { + return lastValueFrom(pendingSubject); + }); + const bootstrapAppFinished = vi.fn(); + + queueBootstrapping(bootstrapApp).then(() => { + bootstrapAppFinished(); + }); + + await vi.waitFor(() => { + assert(bootstrapApp.mock.calls.length === 1, 'bootstrapApp should have been called once'); + }); + + expect(bootstrapApp).toHaveBeenCalled(); + expect(bootstrapAppFinished).not.toHaveBeenCalled(); + + pendingSubject.next(); + pendingSubject.complete(); + + await vi.waitFor(() => { + assert( + bootstrapAppFinished.mock.calls.length === 1, + 'bootstrapApp should have been called once' + ); + }); + + expect(bootstrapAppFinished).toHaveBeenCalled(); + }); + + it('should prevent following tasks, until the preview tasks are complete', async () => { + const pendingSubject = new Subject(); + const bootstrapApp = vi.fn().mockImplementation(async () => { + return lastValueFrom(pendingSubject); + }); + const bootstrapAppFinished = vi.fn(); + + const pendingSubject2 = new Subject(); + const bootstrapApp2 = vi.fn().mockImplementation(async () => { + return lastValueFrom(pendingSubject2); + }); + const bootstrapAppFinished2 = vi.fn(); + + const pendingSubject3 = new Subject(); + const bootstrapApp3 = vi.fn().mockImplementation(async () => { + return lastValueFrom(pendingSubject3); + }); + const bootstrapAppFinished3 = vi.fn(); + + queueBootstrapping(bootstrapApp).then(bootstrapAppFinished); + queueBootstrapping(bootstrapApp2).then(bootstrapAppFinished2); + queueBootstrapping(bootstrapApp3).then(bootstrapAppFinished3); + + await vi.waitFor(() => { + assert(bootstrapApp.mock.calls.length === 1, 'bootstrapApp should have been called once'); + }); + + 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 vi.waitFor(() => { + assert(bootstrapApp2.mock.calls.length === 1, 'bootstrapApp2 should have been called once'); + }); + + 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 vi.waitFor(() => { + assert(bootstrapApp3.mock.calls.length === 1, 'bootstrapApp3 should have been called once'); + }); + + 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 vi.waitFor(() => { + assert( + bootstrapAppFinished3.mock.calls.length === 1, + 'bootstrapAppFinished3 should have been called once' + ); + }); + + expect(bootstrapApp).toHaveReturnedTimes(1); + expect(bootstrapAppFinished).toHaveBeenCalled(); + expect(bootstrapApp2).toHaveReturnedTimes(1); + expect(bootstrapAppFinished2).toHaveBeenCalled(); + expect(bootstrapApp3).toHaveReturnedTimes(1); + expect(bootstrapAppFinished3).toHaveBeenCalled(); + }); + + it('should throw and continue next bootstrap on error', async () => { + const pendingSubject = new Subject(); + const bootstrapApp = vi.fn().mockImplementation(async () => { + return lastValueFrom(pendingSubject); + }); + const bootstrapAppFinished = vi.fn(); + const bootstrapAppError = vi.fn(); + + const pendingSubject2 = new Subject(); + const bootstrapApp2 = vi.fn().mockImplementation(async () => { + return lastValueFrom(pendingSubject2); + }); + const bootstrapAppFinished2 = vi.fn(); + const bootstrapAppError2 = vi.fn(); + + queueBootstrapping(bootstrapApp).then(bootstrapAppFinished).catch(bootstrapAppError); + queueBootstrapping(bootstrapApp2).then(bootstrapAppFinished2).catch(bootstrapAppError2); + + 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(); + + pendingSubject.error(new Error('test error')); + + await vi.waitFor(() => { + assert( + bootstrapAppError.mock.calls.length === 1, + 'bootstrapAppError should have been called once' + ); + }); + + 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 vi.waitFor(() => { + assert( + bootstrapAppFinished2.mock.calls.length === 1, + 'bootstrapAppFinished2 should have been called once' + ); + }); + + expect(bootstrapApp).toHaveBeenCalledTimes(1); + expect(bootstrapAppFinished).not.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/BootstrapQueue.ts b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts new file mode 100644 index 000000000000..20b81c860413 --- /dev/null +++ b/code/frameworks/angular/src/client/angular-beta/utils/BootstrapQueue.ts @@ -0,0 +1,61 @@ +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 + */ + } +}; + +/** + * 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)); + + if (!isProcessing) { + processQueue(); + } + }); +}; + +const processQueue = async () => { + isProcessing = true; + + while (queue.length > 0) { + const bootstrappingFn = queue.shift(); + if (bootstrappingFn) { + await bootstrappingFn(); + await resetCompiledComponents(); + } + } + + isProcessing = false; +};