Merge pull request #16517 from storybookjs/16048-sync-promise

Core: Use synchronous promises to "fake" promises for sync code

addons/storyshots/storyshots-core/src/api/index.ts

@@ -6,7 +6,7 @@ import integrityTest from './integrityTestTemplate';
 import loadFramework from '../frameworks/frameworkLoader';
 import { StoryshotsOptions } from './StoryshotsOptions';
 
-const { describe } = global;
+const { describe, window: globalWindow } = global;
 global.STORYBOOK_REACT_CLASSES = global.STORYBOOK_REACT_CLASSES || {};
 
 type TestMethod = 'beforeAll' | 'beforeEach' | 'afterEach' | 'afterAll';
@@ -48,55 +48,62 @@ function testStorySnapshots(options: StoryshotsOptions = {}) {
     stories2snapsConverter,
   };
 
-  const data = storybook.raw().reduce(
-    (acc, item) => {
-      if (storyNameRegex && !item.name.match(storyNameRegex)) {
-        return acc;
-      }
-
-      if (storyKindRegex && !item.kind.match(storyKindRegex)) {
-        return acc;
-      }
-
-      const { kind, storyFn: render, parameters } = item;
-      const existing = acc.find((i: any) => i.kind === kind);
-      const { fileName } = item.parameters;
-
-      if (!isDisabled(parameters.storyshots)) {
-        if (existing) {
-          existing.children.push({ ...item, render, fileName });
-        } else {
-          acc.push({
-            kind,
-            children: [{ ...item, render, fileName }],
-          });
+  // NOTE: as the store + preview's initialization process entirely uses
+  // `SychronousPromise`s in the v6 store case, the callback to the `then()` here
+  // will run *immediately* (in the same tick), and thus the `snapshotsTests`, and
+  // subsequent calls to `it()` etc will all happen within this tick, which is required
+  // by Jest (cannot add tests asynchronously)
+  globalWindow.__STORYBOOK_STORY_STORE__.initializationPromise.then(() => {
+    const data = storybook.raw().reduce(
+      (acc, item) => {
+        if (storyNameRegex && !item.name.match(storyNameRegex)) {
+          return acc;
         }
-      }
-      return acc;
-    },
-    [] as {
-      kind: string;
-      children: any[];
-    }[]
-  );
 
-  if (data.length) {
-    callTestMethodGlobals(testMethod);
+        if (storyKindRegex && !item.kind.match(storyKindRegex)) {
+          return acc;
+        }
 
-    snapshotsTests({
-      data,
-      asyncJest,
-      suite,
-      framework,
-      testMethod,
-      testMethodParams,
-      snapshotSerializers,
-    });
+        const { kind, storyFn: render, parameters } = item;
+        const existing = acc.find((i: any) => i.kind === kind);
+        const { fileName } = item.parameters;
 
-    integrityTest(integrityOptions, stories2snapsConverter);
-  } else {
-    throw new Error('storyshots found 0 stories');
-  }
+        if (!isDisabled(parameters.storyshots)) {
+          if (existing) {
+            existing.children.push({ ...item, render, fileName });
+          } else {
+            acc.push({
+              kind,
+              children: [{ ...item, render, fileName }],
+            });
+          }
+        }
+        return acc;
+      },
+      [] as {
+        kind: string;
+        children: any[];
+      }[]
+    );
+
+    if (data.length) {
+      callTestMethodGlobals(testMethod);
+
+      snapshotsTests({
+        data,
+        asyncJest,
+        suite,
+        framework,
+        testMethod,
+        testMethodParams,
+        snapshotSerializers,
+      });
+
+      integrityTest(integrityOptions, stories2snapsConverter);
+    } else {
+      throw new Error('storyshots found 0 stories');
+    }
+  });
 }
 
 export default testStorySnapshots;

lib/client-api/package.json

@@ -57,6 +57,7 @@
     "qs": "^6.10.0",
     "regenerator-runtime": "^0.13.7",
     "store2": "^2.12.0",
+    "synchronous-promise": "^2.0.15",
     "ts-dedent": "^2.0.0",
     "util-deprecate": "^1.0.2"
   },

lib/client-api/src/StoryStoreFacade.ts

@@ -1,5 +1,6 @@
 import global from 'global';
 import dedent from 'ts-dedent';
+import { SynchronousPromise } from 'synchronous-promise';
 import {
   StoryId,
   AnyFramework,
@@ -59,9 +60,11 @@ export class StoryStoreFacade<TFramework extends AnyFramework> {
   // This doesn't actually import anything because the client-api loads fully
   // on startup, but this is a shim after all.
   importFn(path: Path) {
-    const moduleExports = this.csfExports[path];
-    if (!moduleExports) throw new Error(`Unknown path: ${path}`);
-    return moduleExports;
+    return SynchronousPromise.resolve().then(() => {
+      const moduleExports = this.csfExports[path];
+      if (!moduleExports) throw new Error(`Unknown path: ${path}`);
+      return moduleExports;
+    });
   }
 
   getStoryIndex(store: StoryStore<TFramework>) {

lib/preview-web/package.json

@@ -52,6 +52,7 @@
     "lodash": "^4.17.20",
     "qs": "^6.10.0",
     "regenerator-runtime": "^0.13.7",
+    "synchronous-promise": "^2.0.15",
     "ts-dedent": "^2.0.0",
     "unfetch": "^4.2.0",
     "util-deprecate": "^1.0.2"

lib/preview-web/src/PreviewWeb.tsx

@@ -1,8 +1,9 @@
 import deprecate from 'util-deprecate';
 import dedent from 'ts-dedent';
+import global from 'global';
+import { SynchronousPromise } from 'synchronous-promise';
 import Events, { IGNORED_EXCEPTION } from '@storybook/core-events';
 import { logger } from '@storybook/client-logger';
-import global from 'global';
 import { addons, Channel } from '@storybook/addons';
 import {
   AnyFramework,
@@ -50,6 +51,7 @@ function createController() {
 }
 
 export type RenderPhase = 'loading' | 'rendering' | 'playing' | 'completed' | 'aborted' | 'errored';
+type PromiseLike<T> = Promise<T> | SynchronousPromise<T>;
 type MaybePromise<T> = Promise<T> | T;
 type StoryCleanupFn = () => MaybePromise<void>;
 
@@ -92,6 +94,12 @@ export class PreviewWeb<TFramework extends AnyFramework> {
     );
   }
 
+  // NOTE: the reason that the preview and store's initialization code is written in a promise
+  // style and not `async-await`, and the use of `SynchronousPromise`s is in order to allow
+  // storyshots to immediately call `raw()` on the store without waiting for a later tick.
+  // (Even simple things like `Promise.resolve()` and `await` involve the callback happening
+  // in the next promise "tick").
+  // See the comment in `storyshots-core/src/api/index.ts` for more detail.
   initialize({
     getStoryIndex,
     importFn,
@@ -102,42 +110,44 @@ export class PreviewWeb<TFramework extends AnyFramework> {
     getStoryIndex?: () => StoryIndex;
     importFn: ModuleImportFn;
     getProjectAnnotations: () => WebProjectAnnotations<TFramework>;
-  }): MaybePromise<void> {
+  }): PromiseLike<void> {
     this.storyStore.setProjectAnnotations(
       this.getProjectAnnotationsOrRenderError(getProjectAnnotations) || {}
     );
 
     this.setupListeners();
 
+    let storyIndexPromise: PromiseLike<StoryIndex>;
     if (FEATURES?.storyStoreV7) {
       this.indexClient = new StoryIndexClient();
-      return this.indexClient
-        .fetch()
-        .then((storyIndex: StoryIndex) => {
-          this.storyStore.initialize({
+      storyIndexPromise = this.indexClient.fetch();
+    } else {
+      if (!getStoryIndex) {
+        throw new Error('No `getStoryIndex` passed defined in v6 mode');
+      }
+      storyIndexPromise = SynchronousPromise.resolve().then(getStoryIndex);
+    }
+
+    return storyIndexPromise
+      .then((storyIndex: StoryIndex) => {
+        return this.storyStore
+          .initialize({
             storyIndex,
             importFn,
-            cache: false,
+            cache: !FEATURES?.storyStoreV7,
+          })
+          .then(() => {
+            if (!FEATURES?.storyStoreV7) {
+              this.channel.emit(Events.SET_STORIES, this.storyStore.getSetStoriesPayload());
+            }
+
+            this.setGlobalsAndRenderSelection();
           });
-          return this.setGlobalsAndRenderSelection();
-        })
-        .catch((err) => {
-          logger.warn(err);
-          this.renderPreviewEntryError(err);
-        });
-    }
-
-    if (!getStoryIndex) {
-      throw new Error('No `getStoryIndex` passed defined in v6 mode');
-    }
-    this.storyStore.initialize({
-      storyIndex: getStoryIndex(),
-      importFn,
-      cache: true,
-    });
-    this.channel.emit(Events.SET_STORIES, this.storyStore.getSetStoriesPayload());
-
-    return this.setGlobalsAndRenderSelection();
+      })
+      .catch((err) => {
+        logger.warn(err);
+        this.renderPreviewEntryError(err);
+      });
   }
 
   getProjectAnnotationsOrRenderError(
@@ -391,9 +401,7 @@ export class PreviewWeb<TFramework extends AnyFramework> {
   async renderDocs({ story }: { story: Story<TFramework> }) {
     const { id, title, name } = story;
     const element = this.view.prepareForDocs();
-    const csfFile: CSFFile<TFramework> = await this.storyStore.loadCSFFileByStoryId(id, {
-      sync: false,
-    });
+    const csfFile: CSFFile<TFramework> = await this.storyStore.loadCSFFileByStoryId(id);
     const docsContext = {
       id,
       title,

lib/store/package.json

@@ -52,6 +52,7 @@
     "regenerator-runtime": "^0.13.7",
     "slash": "^3.0.0",
     "stable": "^0.1.8",
+    "synchronous-promise": "^2.0.15",
     "ts-dedent": "^2.0.0",
     "util-deprecate": "^1.0.2"
   },

lib/store/src/StoryStore.test.ts

@@ -186,7 +186,7 @@ describe('StoryStore', () => {
       expect(prepareStory).toHaveBeenCalledTimes(1);
 
       await store.onStoriesChanged({
-        importFn: () => ({
+        importFn: async () => ({
           ...componentOneExports,
           c: { args: { foo: 'c' } },
         }),
@@ -257,7 +257,7 @@ describe('StoryStore', () => {
       expect(importFn).toHaveBeenCalledWith(storyIndex.stories['component-one--a'].importPath);
 
       const newImportPath = './src/ComponentOne-new.stories.js';
-      const newImportFn = jest.fn(() => componentOneExports);
+      const newImportFn = jest.fn(async () => componentOneExports);
       await store.onStoriesChanged({
         importFn: newImportFn,
         storyIndex: {
@@ -287,7 +287,7 @@ describe('StoryStore', () => {
       expect(importFn).toHaveBeenCalledWith(storyIndex.stories['component-one--a'].importPath);
 
       const newImportPath = './src/ComponentOne-new.stories.js';
-      const newImportFn = jest.fn(() => componentOneExports);
+      const newImportFn = jest.fn(async () => componentOneExports);
       await store.onStoriesChanged({
         importFn: newImportFn,
         storyIndex: {
@@ -350,7 +350,7 @@ describe('StoryStore', () => {
       store.setProjectAnnotations(projectAnnotations);
       store.initialize({ storyIndex, importFn, cache: false });
 
-      const csfFile = await store.loadCSFFileByStoryId('component-one--a', { sync: false });
+      const csfFile = await store.loadCSFFileByStoryId('component-one--a');
       const stories = store.componentStoriesFromCSFFile({ csfFile });
 
       expect(stories).toHaveLength(2);
@@ -426,7 +426,7 @@ describe('StoryStore', () => {
       store.initialize({ storyIndex, importFn, cache: false });
 
       importFn.mockClear();
-      const csfFiles = await store.loadAllCSFFiles(false);
+      const csfFiles = await store.loadAllCSFFiles();
 
       expect(Object.keys(csfFiles)).toEqual([
         './src/ComponentOne.stories.js',
@@ -448,7 +448,7 @@ describe('StoryStore', () => {
       const store = new StoryStore();
       store.setProjectAnnotations(projectAnnotations);
       store.initialize({ storyIndex, importFn, cache: false });
-      await store.cacheAllCSFFiles(false);
+      await store.cacheAllCSFFiles();
 
       expect(store.extract()).toMatchInlineSnapshot(`
         Object {
@@ -574,7 +574,7 @@ describe('StoryStore', () => {
         importFn: docsOnlyImportFn,
         cache: false,
       });
-      await store.cacheAllCSFFiles(false);
+      await store.cacheAllCSFFiles();
 
       expect(Object.keys(store.extract())).toEqual(['component-one--b', 'component-two--c']);
 
@@ -591,7 +591,7 @@ describe('StoryStore', () => {
       const store = new StoryStore();
       store.setProjectAnnotations(projectAnnotations);
       store.initialize({ storyIndex, importFn, cache: false });
-      await store.cacheAllCSFFiles(false);
+      await store.cacheAllCSFFiles();
 
       expect(store.raw()).toMatchInlineSnapshot(`
         Array [
@@ -713,7 +713,7 @@ describe('StoryStore', () => {
       const store = new StoryStore();
       store.setProjectAnnotations(projectAnnotations);
       store.initialize({ storyIndex, importFn, cache: false });
-      await store.cacheAllCSFFiles(false);
+      await store.cacheAllCSFFiles();
 
       expect(store.getSetStoriesPayload()).toMatchInlineSnapshot(`
         Object {
@@ -847,7 +847,7 @@ describe('StoryStore', () => {
         const store = new StoryStore();
         store.setProjectAnnotations(projectAnnotations);
         store.initialize({ storyIndex, importFn, cache: false });
-        await store.cacheAllCSFFiles(false);
+        await store.cacheAllCSFFiles();
 
         expect(store.getStoriesJsonData()).toMatchInlineSnapshot(`
           Object {
@@ -903,7 +903,7 @@ describe('StoryStore', () => {
         const store = new StoryStore();
         store.setProjectAnnotations(projectAnnotations);
         store.initialize({ storyIndex, importFn, cache: false });
-        await store.cacheAllCSFFiles(false);
+        await store.cacheAllCSFFiles();
 
         expect(store.getStoriesJsonData()).toMatchInlineSnapshot(`
           Object {

lib/store/src/StoryStore.ts

@@ -12,6 +12,7 @@ import {
 import mapValues from 'lodash/mapValues';
 import pick from 'lodash/pick';
 import global from 'global';
+import { SynchronousPromise } from 'synchronous-promise';
 
 import { StoryIndexStore } from './StoryIndexStore';
 import { ArgsStore } from './ArgsStore';
@@ -25,7 +26,6 @@ import {
   NormalizedProjectAnnotations,
   Path,
   ExtractOptions,
-  ModuleExports,
   BoundStory,
   StoryIndex,
   StoryIndexEntry,
@@ -38,8 +38,6 @@ import { inferControls } from './inferControls';
 
 const { FEATURES } = global;
 
-type MaybePromise<T> = Promise<T> | T;
-
 // TODO -- what are reasonable values for these?
 const CSF_CACHE_SIZE = 1000;
 const STORY_CACHE_SIZE = 10000;
@@ -101,7 +99,7 @@ export class StoryStore<TFramework extends AnyFramework> {
     this.prepareStoryWithCache = memoize(STORY_CACHE_SIZE)(prepareStory) as typeof prepareStory;
 
     // We cannot call `loadStory()` until we've been initialized properly. But we can wait for it.
-    this.initializationPromise = new Promise((resolve) => {
+    this.initializationPromise = new SynchronousPromise((resolve) => {
       this.resolveInitializationPromise = resolve;
     });
   }
@@ -122,14 +120,14 @@ export class StoryStore<TFramework extends AnyFramework> {
     storyIndex: StoryIndex;
     importFn: ModuleImportFn;
     cache?: boolean;
-  }): void {
+  }): PromiseLike<void> {
     this.storyIndex = new StoryIndexStore(storyIndex);
     this.importFn = importFn;
 
     // We don't need the cache to be loaded to call `loadStory`, we just need the index ready
     this.resolveInitializationPromise();
 
-    if (cache) this.cacheAllCSFFiles(true);
+    return cache ? this.cacheAllCSFFiles() : SynchronousPromise.resolve();
   }
 
   // This means that one of the CSF files has changed.
@@ -145,93 +143,41 @@ export class StoryStore<TFramework extends AnyFramework> {
   }) {
     if (importFn) this.importFn = importFn;
     if (storyIndex) this.storyIndex.stories = storyIndex.stories;
-
-    if (this.cachedCSFFiles) {
-      await this.cacheAllCSFFiles(false);
-    }
+    if (this.cachedCSFFiles) await this.cacheAllCSFFiles();
   }
 
   // To load a single CSF file to service a story we need to look up the importPath in the index
-  loadCSFFileByStoryId(storyId: StoryId, options: { sync: false }): Promise<CSFFile<TFramework>>;
-
-  loadCSFFileByStoryId(storyId: StoryId, options: { sync: true }): CSFFile<TFramework>;
-
-  loadCSFFileByStoryId(
-    storyId: StoryId,
-    { sync = false }: { sync?: boolean } = {}
-  ): MaybePromise<CSFFile<TFramework>> {
+  loadCSFFileByStoryId(storyId: StoryId): PromiseLike<CSFFile<TFramework>> {
     const { importPath, title } = this.storyIndex.storyIdToEntry(storyId);
-    const moduleExportsOrPromise = this.importFn(importPath);
-
-    if (sync) {
-      // NOTE: This isn't a totally reliable way to check if an object is a promise
-      // (doesn't work in Angular), but it's just to help users in an error case.
-      if (Promise.resolve(moduleExportsOrPromise) === moduleExportsOrPromise) {
-        throw new Error(
-          `importFn() returned a promise, did you pass an async version then call initialize({sync: true})?`
-        );
-      }
-
-      // We pass the title in here as it may have been generated by autoTitle on the server.
-      return this.processCSFFileWithCache(moduleExportsOrPromise as ModuleExports, title);
-    }
-
-    return Promise.resolve(moduleExportsOrPromise).then((moduleExports) =>
+    return this.importFn(importPath).then((moduleExports) =>
       // We pass the title in here as it may have been generated by autoTitle on the server.
       this.processCSFFileWithCache(moduleExports, title)
     );
   }
 
-  loadAllCSFFiles(sync: false): Promise<StoryStore<TFramework>['cachedCSFFiles']>;
-
-  loadAllCSFFiles(sync: true): StoryStore<TFramework>['cachedCSFFiles'];
-
-  loadAllCSFFiles(sync: boolean): MaybePromise<StoryStore<TFramework>['cachedCSFFiles']> {
+  loadAllCSFFiles(): PromiseLike<StoryStore<TFramework>['cachedCSFFiles']> {
     const importPaths: Record<Path, StoryId> = {};
     Object.entries(this.storyIndex.stories).forEach(([storyId, { importPath }]) => {
       importPaths[importPath] = storyId;
     });
 
-    const csfFileList = Object.entries(importPaths).map(([importPath, storyId]) => ({
-      importPath,
-      csfFileOrPromise: sync
-        ? this.loadCSFFileByStoryId(storyId, { sync: true })
-        : this.loadCSFFileByStoryId(storyId, { sync: false }),
-    }));
+    const csfFilePromiseList = Object.entries(importPaths).map(([importPath, storyId]) =>
+      this.loadCSFFileByStoryId(storyId).then((csfFile) => ({
+        importPath,
+        csfFile,
+      }))
+    );
 
-    function toObject(list: { importPath: Path; csfFile: CSFFile<TFramework> }[]) {
-      return list.reduce((acc, { importPath, csfFile }) => {
+    return SynchronousPromise.all(csfFilePromiseList).then((list) =>
+      list.reduce((acc, { importPath, csfFile }) => {
         acc[importPath] = csfFile;
         return acc;
-      }, {} as Record<Path, CSFFile<TFramework>>);
-    }
-
-    if (sync) {
-      return toObject(
-        csfFileList.map(({ importPath, csfFileOrPromise }) => ({
-          importPath,
-          csfFile: csfFileOrPromise,
-        })) as { importPath: Path; csfFile: CSFFile<TFramework> }[]
-      );
-    }
-    return Promise.all(
-      csfFileList.map(async ({ importPath, csfFileOrPromise }) => ({
-        importPath,
-        csfFile: await csfFileOrPromise,
-      }))
-    ).then(toObject);
+      }, {} as Record<Path, CSFFile<TFramework>>)
+    );
   }
 
-  cacheAllCSFFiles(sync: false): Promise<void>;
-
-  cacheAllCSFFiles(sync: true): void;
-
-  cacheAllCSFFiles(sync: boolean): MaybePromise<void> {
-    if (sync) {
-      this.cachedCSFFiles = this.loadAllCSFFiles(true);
-      return null;
-    }
-    return this.loadAllCSFFiles(false).then((csfFiles) => {
+  cacheAllCSFFiles(): PromiseLike<void> {
+    return this.loadAllCSFFiles().then((csfFiles) => {
       this.cachedCSFFiles = csfFiles;
     });
   }
@@ -239,7 +185,7 @@ export class StoryStore<TFramework extends AnyFramework> {
   // Load the CSF file for a story and prepare the story from it and the project annotations.
   async loadStory({ storyId }: { storyId: StoryId }): Promise<Story<TFramework>> {
     await this.initializationPromise;
-    const csfFile = await this.loadCSFFileByStoryId(storyId, { sync: false });
+    const csfFile = await this.loadCSFFileByStoryId(storyId);
     return this.storyFromCSFFile({ storyId, csfFile });
   }
 

lib/store/src/types.ts

@@ -1,3 +1,4 @@
+import { SynchronousPromise } from 'synchronous-promise';
 import {
   DecoratorFunction,
   Args,
@@ -25,7 +26,8 @@ import {
 export type { StoryId, Parameters };
 export type Path = string;
 export type ModuleExports = Record<string, any>;
-export type ModuleImportFn = (path: Path) => Promise<ModuleExports> | ModuleExports;
+type PromiseLike<T> = Promise<T> | SynchronousPromise<T>;
+export type ModuleImportFn = (path: Path) => PromiseLike<ModuleExports>;
 
 export type NormalizedProjectAnnotations<
   TFramework extends AnyFramework = AnyFramework

yarn.lock

@@ -7895,6 +7895,7 @@ __metadata:
     qs: ^6.10.0
     regenerator-runtime: ^0.13.7
     store2: ^2.12.0
+    synchronous-promise: ^2.0.15
     ts-dedent: ^2.0.0
     util-deprecate: ^1.0.2
   peerDependencies:
@@ -8659,6 +8660,7 @@ __metadata:
     lodash: ^4.17.20
     qs: ^6.10.0
     regenerator-runtime: ^0.13.7
+    synchronous-promise: ^2.0.15
     ts-dedent: ^2.0.0
     unfetch: ^4.2.0
     util-deprecate: ^1.0.2
@@ -9084,6 +9086,7 @@ __metadata:
     regenerator-runtime: ^0.13.7
     slash: ^3.0.0
     stable: ^0.1.8
+    synchronous-promise: ^2.0.15
     ts-dedent: ^2.0.0
     util-deprecate: ^1.0.2
   languageName: unknown
@@ -42571,6 +42574,13 @@ resolve@1.19.0:
   languageName: node
   linkType: hard
 
+"synchronous-promise@npm:^2.0.15":
+  version: 2.0.15
+  resolution: "synchronous-promise@npm:2.0.15"
+  checksum: 967778e7570dc496d7630a89db3bada38876574797c9b272ee50f6ecd7afcebf450268b4bb48a84274d213ab9fd4865dbcc6edeb279f9ecaddf189d5446cbe43
+  languageName: node
+  linkType: hard
+
 "table@npm:^5.2.3":
   version: 5.4.6
   resolution: "table@npm:5.4.6"