Core: Only render once when changing args while loading/rendering

code/e2e-tests/preview-web.spec.ts → code/e2e-tests/preview-api.spec.ts

@@ -5,7 +5,9 @@ import { SbPage } from './util';
 const storybookUrl = process.env.STORYBOOK_URL || 'http://localhost:8001';
 const templateName = process.env.STORYBOOK_TEMPLATE_NAME || '';
 
-test.describe('preview-web', () => {
+const wait = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
+
+test.describe('preview-api', () => {
   test.beforeEach(async ({ page }) => {
     await page.goto(storybookUrl);
 
@@ -50,4 +52,40 @@ test.describe('preview-web', () => {
     await sbPage.previewRoot().getByRole('button').getByText('Submit').first().press('Alt+s');
     await expect(sbPage.page.locator('.sidebar-container')).not.toBeVisible();
   });
+
+  // if rerenders were interleaved the button would have rendered "Error: Interleaved loaders. Changed arg"
+  test('should only render once at a time when rapidly changing args', async ({ page }) => {
+    const sbPage = new SbPage(page);
+    await sbPage.navigateToStory('lib/preview-api/rendering', 'slow-loader');
+
+    const root = sbPage.previewRoot();
+
+    const labelControl = await sbPage.page.locator('#control-label');
+
+    await expect(root.getByText('Loaded. Click me')).toBeVisible();
+    await expect(labelControl).toBeVisible();
+
+    await labelControl.fill('');
+    await labelControl.type('Changed arg', { delay: 50 });
+    await labelControl.blur();
+
+    await expect(root.getByText('Loaded. Changed arg')).toBeVisible();
+  });
+
+  test('should reload plage when remounting while loading', async ({ page }) => {
+    const sbPage = new SbPage(page);
+    await sbPage.navigateToStory('lib/preview-api/rendering', 'slow-loader');
+
+    const root = sbPage.previewRoot();
+
+    await expect(root.getByText('Loaded. Click me')).toBeVisible();
+
+    await sbPage.page.getByRole('button', { name: 'Remount component' }).click();
+    await wait(200);
+    await sbPage.page.getByRole('button', { name: 'Remount component' }).click();
+
+    // the loading spinner indicates the iframe is being fully reloaded
+    await expect(sbPage.previewIframe().locator('.sb-preparing-story > .sb-loader')).toBeVisible();
+    await expect(root.getByText('Loaded. Click me')).toBeVisible();
+  });
 });

code/lib/preview-api/src/modules/preview-web/render/StoryRender.test.ts

@@ -81,6 +81,58 @@ describe('StoryRender', () => {
     expect(story.playFunction).not.toHaveBeenCalled();
   });
 
+  it('only rerenders once when triggered multiple times while pending', async () => {
+    // Arrange - setup StoryRender and async gate blocking applyLoaders
+    const [loaderGate, openLoaderGate] = createGate();
+    const story = {
+      id: 'id',
+      title: 'title',
+      name: 'name',
+      tags: [],
+      applyLoaders: vi.fn(() => loaderGate),
+      unboundStoryFn: vi.fn(),
+      playFunction: vi.fn(),
+      prepareContext: vi.fn(),
+    };
+    const store = { getStoryContext: () => ({}), cleanupStory: vi.fn() };
+    const renderToScreen = vi.fn();
+    const render = new StoryRender(
+      new Channel({}),
+      store as any,
+      renderToScreen,
+      {} as any,
+      entry.id,
+      'story',
+      { autoplay: true },
+      story as any
+    );
+    // Arrange - render (blocked by loaders)
+    render.renderToElement({} as any);
+    expect(story.applyLoaders).toHaveBeenCalledOnce();
+    expect(render.phase).toBe('loading');
+
+    // Act - rerender 3x
+    render.rerender();
+    render.rerender();
+    render.rerender();
+
+    // Assert - still loading, not yet rendered
+    expect(story.applyLoaders).toHaveBeenCalledOnce();
+    expect(render.phase).toBe('loading');
+    expect(renderToScreen).not.toHaveBeenCalled();
+
+    // Act - finish loading
+    openLoaderGate();
+
+    // Assert - loaded and rendered twice, played once
+    await vi.waitFor(async () => {
+      console.log(render.phase);
+      expect(story.applyLoaders).toHaveBeenCalledTimes(2);
+      expect(renderToScreen).toHaveBeenCalledTimes(2);
+      expect(story.playFunction).toHaveBeenCalledOnce();
+    });
+  });
+
   describe('teardown', () => {
     it('throws PREPARE_ABORTED if torndown during prepare', async () => {
       const [importGate, openImportGate] = createGate();

code/lib/preview-api/src/modules/preview-web/render/StoryRender.ts

@@ -56,6 +56,8 @@ export class StoryRender<TRenderer extends Renderer> implements Render<TRenderer
 
   private notYetRendered = true;
 
+  private rerenderEnqueued = false;
+
   public disableKeyListeners = false;
 
   private teardownRender: TeardownRenderToCanvas = () => {};
@@ -268,10 +270,27 @@ export class StoryRender<TRenderer extends Renderer> implements Render<TRenderer
       this.phase = 'errored';
       this.callbacks.showException(err as Error);
     }
+
+    // If a rerender was enqueued during the render, clear the queue and render again
+    if (this.rerenderEnqueued) {
+      this.rerenderEnqueued = false;
+      this.render();
+    }
   }
 
+  /**
+   * Rerender the story.
+   * If the story is currently pending (loading/rendering), the rerender will be enqueued,
+   * and will be executed after the current render is completed.
+   * Rerendering while playing will not be enqueued, and will be executed immediately, to support
+   * rendering args changes while playing.
+   */
   async rerender() {
-    return this.render();
+    if (this.isPending() && this.phase !== 'playing') {
+      this.rerenderEnqueued = true;
+    } else {
+      return this.render();
+    }
   }
 
   async remount() {

code/lib/preview-api/template/stories/rendering.stories.ts

@@ -66,3 +66,31 @@ export const ChangeArgs = {
     await expect(button).toHaveFocus();
   },
 };
+
+let loadedLabel = 'Initial';
+
+/**
+ * This story demonstrates what happens when rendering (loaders) have side effects, and can possibly interleave with each other
+ * Triggering multiple force remounts quickly should only result in a single remount in the end
+ * and the label should be 'Loaded. Click Me' at the end. If loaders are interleaving it would result in a label of 'Error: Interleaved loaders. Click Me'
+ * Similarly, changing args rapidly should only cause one rerender at a time, producing the same result.
+ */
+export const SlowLoader = {
+  loaders: [
+    async () => {
+      loadedLabel = 'Loading...';
+      await new Promise((resolve) => setTimeout(resolve, 1000));
+      loadedLabel = loadedLabel === 'Loading...' ? 'Loaded.' : 'Error: Interleaved loaders.';
+      return { label: loadedLabel };
+    },
+  ],
+  decorators: [
+    (storyFn: any, context: any) =>
+      storyFn({
+        args: {
+          ...context.args,
+          label: `${context.loaded.label} ${context.args.label}`,
+        },
+      }),
+  ],
+};