Ensure that we do not render a story twice if re-rendered during prep…

…arign
storybookjs · Mar 1, 2022 · f2848ec · f2848ec
1 parent 397d4f8
commit f2848ec
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 11 deletions.
diff --git a/lib/preview-web/src/PreviewWeb.test.ts b/lib/preview-web/src/PreviewWeb.test.ts
@@ -1320,6 +1320,39 @@ describe('PreviewWeb', () => {
         await waitForQuiescence();
         expect(projectAnnotations.renderToDOM).not.toHaveBeenCalled();
       });
+
+      // For https://github.com/storybookjs/storybook/issues/17214
+      it('does NOT render a second time if preparing', async () => {
+        document.location.search = '?id=component-one--a';
+
+        const [gate, openGate] = createGate();
+        importFn.mockImplementationOnce(async (...args) => {
+          await gate;
+          return importFn(...args);
+        });
+        const preview = new PreviewWeb();
+
+        // We can't wait for the initialize function, as it waits for `renderSelection()`
+        // which prepares, but it does emit `CURRENT_STORY_WAS_SET` right before that
+        preview.initialize({ importFn, getProjectAnnotations });
+        await waitForEvents([Events.CURRENT_STORY_WAS_SET]);
+
+        mockChannel.emit.mockClear();
+        projectAnnotations.renderToDOM.mockClear();
+        emitter.emit(Events.SET_CURRENT_STORY, {
+          storyId: 'component-one--a',
+          viewMode: 'story',
+        });
+        await waitForRender();
+        expect(projectAnnotations.renderToDOM).toHaveBeenCalledTimes(1);
+
+        mockChannel.emit.mockClear();
+        projectAnnotations.renderToDOM.mockClear();
+        openGate();
+        // The renderToDOM would have been async so we need to wait a tick.
+        await waitForQuiescence();
+        expect(projectAnnotations.renderToDOM).not.toHaveBeenCalled();
+      });
     });
 
     describe('when changing story in story viewMode', () => {

diff --git a/lib/preview-web/src/PreviewWeb.tsx b/lib/preview-web/src/PreviewWeb.tsx
@@ -454,6 +454,8 @@ export class PreviewWeb<TFramework extends AnyFramework> {
       this.view.showPreparingDocs();
     }
 
+    const { currentSelection, currentRender } = this;
+
     const storyRender: PreviewWeb<TFramework>['currentRender'] = new StoryRender<
       HTMLElement,
       TFramework
@@ -469,32 +471,37 @@ export class PreviewWeb<TFramework extends AnyFramework> {
       storyId,
       'story'
     );
+    // We need to store this right away, so if the story changes during
+    // the async `.prepare()` below, we can (potentially) cancel it
+    this.currentSelection = selection;
+    // Note this may be replaced by a docsRender after preparing
+    this.currentRender = storyRender;
 
     try {
       await storyRender.prepare();
     } catch (err) {
-      await this.currentRender?.teardown();
+      await currentRender?.teardown();
       this.currentRender = null;
       this.renderStoryLoadingException(storyId, err);
       return;
     }
-    const implementationChanged = !storyIdChanged && !storyRender.isEqual(this.currentRender);
+    const implementationChanged = !storyIdChanged && !storyRender.isEqual(currentRender);
 
     if (persistedArgs) this.storyStore.args.updateFromPersisted(storyRender.story, persistedArgs);
 
     const { parameters, initialArgs, argTypes, args } = storyRender.context();
 
     // Don't re-render the story if nothing has changed to justify it
-    if (this.currentRender && !storyIdChanged && !implementationChanged && !viewModeChanged) {
+    if (currentRender && !storyIdChanged && !implementationChanged && !viewModeChanged) {
       this.channel.emit(Events.STORY_UNCHANGED, storyId);
       this.view.showMain();
       return;
     }
 
-    await this.currentRender?.teardown({ viewModeChanged });
+    await currentRender?.teardown({ viewModeChanged });
 
     // If we are rendering something new (as opposed to re-rendering the same or first story), emit
-    if (this.currentSelection && (storyIdChanged || viewModeChanged)) {
+    if (currentSelection && (storyIdChanged || viewModeChanged)) {
       this.channel.emit(Events.STORY_CHANGED, storyId);
     }
 
@@ -515,10 +522,6 @@ export class PreviewWeb<TFramework extends AnyFramework> {
       this.channel.emit(Events.STORY_ARGS_UPDATED, { storyId, args });
     }
 
-    // Record the previous selection *before* awaiting the rendering, in cases things change before it is done.
-    this.currentSelection = selection;
-    this.currentRender = storyRender; // may be replaced immedately below
-
     if (selection.viewMode === 'docs' || parameters.docsOnly) {
       this.currentRender = storyRender.toDocsRender();
       this.currentRender.renderToElement(this.view.prepareForDocs(), this.renderStoryToElement);

diff --git a/lib/preview-web/src/StoryRender.ts b/lib/preview-web/src/StoryRender.ts
@@ -95,8 +95,10 @@ export class StoryRender<
       this.story = await this.store.loadStory({ storyId: this.id });
     });
 
-    if (this.abortController.signal.aborted)
+    if (this.abortController.signal.aborted) {
+      this.store.cleanupStory(this.story);
       throw new Error('Story render aborted during preparation');
+    }
   }
 
   // The two story "renders" are equal and have both loaded the same story
@@ -225,7 +227,8 @@ export class StoryRender<
   async teardown(options: {} = {}) {
     this.cancelRender();
 
-    this.store.cleanupStory(this.story);
+    // If the story has loaded, we need to cleanup
+    if (this.story) this.store.cleanupStory(this.story);
 
     // Check if we're done rendering/playing. If not, we may have to reload the page.
     // Wait several ticks that may be needed to handle the abort, then try again.