Vite: Improve handling of preview annotations

code/.storybook/preview.tsx

@@ -118,8 +118,6 @@ const ThemedSetRoot = () => {
   return null;
 };
 
-// eslint-disable-next-line no-underscore-dangle
-const preview = (window as any).__STORYBOOK_PREVIEW__ as PreviewWeb<ReactRenderer>;
 const channel = (window as any).__STORYBOOK_ADDONS_CHANNEL__ as Channel;
 export const loaders = [
   /**
@@ -135,6 +133,8 @@ export const loaders = [
    * The DocsContext will then be added via the decorator below.
    */
   async ({ parameters: { relativeCsfPaths, attached = true } }) => {
+    // eslint-disable-next-line no-underscore-dangle
+    const preview = (window as any).__STORYBOOK_PREVIEW__ as PreviewWeb<ReactRenderer>;
     if (!relativeCsfPaths) {
       return {};
     }

code/builders/builder-vite/package.json

@@ -49,7 +49,9 @@
     "es-module-lexer": "^1.5.0",
     "express": "^4.19.2",
     "find-cache-dir": "^3.0.0",
+    "knitwork": "^1.1.0",
     "magic-string": "^0.30.0",
+    "pathe": "^1.1.2",
     "ts-dedent": "^2.0.0"
   },
   "devDependencies": {

code/builders/builder-vite/src/codegen-modern-iframe-script.test.ts

@@ -0,0 +1,24 @@
+import { describe, expect, it } from 'vitest';
+
+import { generateModernIframeScriptCodeFromPreviews } from './codegen-modern-iframe-script';
+
+const projectRoot = 'projectRoot';
+
+describe('generateModernIframeScriptCodeFromPreviews', () => {
+  it('handle one annotation', async () => {
+    const result = await generateModernIframeScriptCodeFromPreviews({
+      previewAnnotations: ['/user/previewAnnotations'],
+      projectRoot,
+      frameworkName: 'frameworkName',
+    });
+    expect(result).toMatchSnapshot();
+  });
+  it('handle multiple annotations', async () => {
+    const result = await generateModernIframeScriptCodeFromPreviews({
+      previewAnnotations: ['/user/previewAnnotations1', '/user/previewAnnotations2'],
+      projectRoot,
+      frameworkName: 'frameworkName',
+    });
+    expect(result).toMatchSnapshot();
+  });
+});

code/builders/builder-vite/src/codegen-modern-iframe-script.ts

@@ -1,6 +1,10 @@
 import { getFrameworkName, loadPreviewOrConfigFile } from 'storybook/internal/common';
 import type { Options, PreviewAnnotation } from 'storybook/internal/types';
 
+import { genArrayFromRaw, genImport, genSafeVariableName } from 'knitwork';
+import { filename } from 'pathe/utils';
+import { dedent } from 'ts-dedent';
+
 import { processPreviewAnnotation } from './utils/process-preview-annotation';
 import { SB_VIRTUAL_FILES, getResolvedVirtualModuleId } from './virtual-file-names';
 
@@ -14,27 +18,55 @@ export async function generateModernIframeScriptCode(options: Options, projectRo
     [],
     options
   );
-  const previewAnnotationURLs = [...previewAnnotations, previewOrConfigFile]
-    .filter(Boolean)
-    .map((path) => processPreviewAnnotation(path, projectRoot));
+  return generateModernIframeScriptCodeFromPreviews({
+    previewAnnotations: [
+      ...previewAnnotations.map((p) => (typeof p === 'string' ? p : p.absolute)),
+      previewOrConfigFile,
+    ],
+    projectRoot,
+    frameworkName,
+  });
+}
+
+export async function generateModernIframeScriptCodeFromPreviews(options: {
+  previewAnnotations: (string | undefined)[];
+  projectRoot: string;
+  frameworkName: string;
+}) {
+  const { projectRoot, frameworkName } = options;
+  const previewAnnotationURLs = options.previewAnnotations
+    .filter((path) => path !== undefined)
+    .map((path) => processPreviewAnnotation(path as string, projectRoot));
+
+  const variables: string[] = [];
+  const imports: string[] = [];
+  for (const previewAnnotation of previewAnnotationURLs) {
+    const variable =
+      genSafeVariableName(filename(previewAnnotation)).replace(/_(45|46|47)/g, '_') +
+      '_' +
+      hash(previewAnnotation);
+    variables.push(variable);
+    imports.push(genImport(previewAnnotation, { name: '*', as: variable }));
+  }
 
   // This is pulled out to a variable because it is reused in both the initial page load
-  // and the HMR handler.  We don't use the hot.accept callback params because only the changed
-  // modules are provided, the rest are null.  We can just re-import everything again in that case.
-  const getPreviewAnnotationsFunction = `
-  const getProjectAnnotations = async (hmrPreviewAnnotationModules = []) => {
-    const configs = await Promise.all([${previewAnnotationURLs
-      .map(
+  // and the HMR handler.
+  // The `hmrPreviewAnnotationModules` parameter is used to pass the updated modules from HMR.
+  // However, only the changed modules are provided, the rest are null.
+  const getPreviewAnnotationsFunction = dedent`
+  const getProjectAnnotations = (hmrPreviewAnnotationModules = []) => {
+    const configs = ${genArrayFromRaw(
+      variables.map(
         (previewAnnotation, index) =>
-          // Prefer the updated module from an HMR update, otherwise import the original module
-          `hmrPreviewAnnotationModules[${index}] ?? import('${previewAnnotation}')`
-      )
-      .join(',\n')}])
+          // Prefer the updated module from an HMR update, otherwise the original module
+          `hmrPreviewAnnotationModules[${index}] ?? ${previewAnnotation}`
+      ),
+      '  '
+    )}
     return composeConfigs(configs);
   }`;
 
-  // eslint-disable-next-line @typescript-eslint/no-shadow
-  const generateHMRHandler = (frameworkName: string): string => {
+  const generateHMRHandler = (): string => {
     // Web components are not compatible with HMR, so disable HMR, reload page instead.
     if (frameworkName === '@storybook/web-components-vite') {
       return `
@@ -43,19 +75,18 @@ export async function generateModernIframeScriptCode(options: Options, projectRo
       }`.trim();
     }
 
-    return `
+    return dedent`
     if (import.meta.hot) {
       import.meta.hot.accept('${getResolvedVirtualModuleId(SB_VIRTUAL_FILES.VIRTUAL_STORIES_FILE)}', (newModule) => {
-      // importFn has changed so we need to patch the new one in
-      window.__STORYBOOK_PREVIEW__.onStoriesChanged({ importFn: newModule.importFn });
+        // importFn has changed so we need to patch the new one in
+        window.__STORYBOOK_PREVIEW__.onStoriesChanged({ importFn: newModule.importFn });
       });
 
-    import.meta.hot.accept(${JSON.stringify(previewAnnotationURLs)}, (previewAnnotationModules) => {
-      ${getPreviewAnnotationsFunction}
-      // getProjectAnnotations has changed so we need to patch the new one in
-      window.__STORYBOOK_PREVIEW__.onGetProjectAnnotationsChanged({ getProjectAnnotations: () => getProjectAnnotations(previewAnnotationModules) });
-    });
-  }`.trim();
+      import.meta.hot.accept(${JSON.stringify(previewAnnotationURLs)}, (previewAnnotationModules) => {
+        // getProjectAnnotations has changed so we need to patch the new one in
+        window.__STORYBOOK_PREVIEW__.onGetProjectAnnotationsChanged({ getProjectAnnotations: () => getProjectAnnotations(previewAnnotationModules) });
+      });
+    }`.trim();
   };
 
   /**
@@ -66,18 +97,21 @@ export async function generateModernIframeScriptCode(options: Options, projectRo
    *
    * @todo Inline variable and remove `noinspection`
    */
-  const code = `
+  const code = dedent`
   import { composeConfigs, PreviewWeb, ClientApi } from 'storybook/internal/preview-api';
   import '${SB_VIRTUAL_FILES.VIRTUAL_ADDON_SETUP_FILE}';
   import { importFn } from '${SB_VIRTUAL_FILES.VIRTUAL_STORIES_FILE}';
-
-    ${getPreviewAnnotationsFunction}
+  ${imports.join('\n')}
+  ${getPreviewAnnotationsFunction}
 
-    window.__STORYBOOK_PREVIEW__ = window.__STORYBOOK_PREVIEW__ || new PreviewWeb(importFn, getProjectAnnotations);
-    
-    window.__STORYBOOK_STORY_STORE__ = window.__STORYBOOK_STORY_STORE__ || window.__STORYBOOK_PREVIEW__.storyStore;
-    
-    ${generateHMRHandler(frameworkName)};
-    `.trim();
+  window.__STORYBOOK_PREVIEW__ = window.__STORYBOOK_PREVIEW__ || new PreviewWeb(importFn, getProjectAnnotations);
+
+  window.__STORYBOOK_STORY_STORE__ = window.__STORYBOOK_STORY_STORE__ || window.__STORYBOOK_PREVIEW__.storyStore;
+
+  ${generateHMRHandler()};
+  `.trim();
   return code;
 }
+function hash(value: string) {
+  return value.split('').reduce((acc, char) => acc + char.charCodeAt(0), 0);
+}

code/builders/builder-vite/src/optimizeDeps.ts

@@ -100,6 +100,7 @@ const INCLUDE_CANDIDATES = [
   'refractor/lang/typescript.js',
   'refractor/lang/yaml.js',
   'regenerator-runtime/runtime.js',
+  'semver', // TODO: Remove once https://github.com/npm/node-semver/issues/712 is fixed
   'slash',
   'store2',
   'synchronous-promise',

code/builders/builder-vite/src/utils/process-preview-annotation.test.ts

@@ -4,64 +4,71 @@ import { onlyWindows, skipWindows } from '../../../../vitest.helpers';
 import { processPreviewAnnotation } from './process-preview-annotation';
 
 describe('processPreviewAnnotation()', () => {
-  it('should pull the `bare` value from an object', () => {
+  it('should pull the `absolute` value from an object', () => {
     const annotation = {
       bare: '@storybook/addon-links/preview',
       absolute: '/Users/foo/storybook/node_modules/@storybook/addon-links/dist/preview.mjs',
     };
     const url = processPreviewAnnotation(annotation, '/Users/foo/storybook/');
-    expect(url).toBe('@storybook/addon-links/preview');
+    expect(url).toBe('/Users/foo/storybook/node_modules/@storybook/addon-links/dist/preview.mjs');
   });
 
-  it('should convert relative paths into urls', () => {
-    const annotation = './src/stories/components';
+  it('should convert relative paths into absolute paths', () => {
+    const annotation = './src/stories/preview';
     const url = processPreviewAnnotation(annotation, '/Users/foo/storybook/');
-    expect(url).toBe('/src/stories/components');
+    expect(url).toBe('/Users/foo/storybook/src/stories/preview');
   });
 
-  skipWindows(() => {
-    it('should convert absolute filesystem paths into urls relative to project root', () => {
-      const annotation = '/Users/foo/storybook/.storybook/preview.js';
-      const url = processPreviewAnnotation(annotation, '/Users/foo/storybook/');
-      expect(url).toBe('/.storybook/preview.js');
-    });
+  it('should keep absolute filesystem paths', () => {
+    const annotation = '/Users/foo/storybook/.storybook/preview.js';
+    const url = processPreviewAnnotation(annotation, '/Users/foo/storybook/');
+    expect(url).toBe('/Users/foo/storybook/.storybook/preview.js');
+  });
 
-    // TODO: figure out why this fails on windows. Could be related to storybook-metadata.test file altering path.sep
-    it('should convert node_modules into bare paths', () => {
-      const annotation = '/Users/foo/storybook/node_modules/storybook-addon/preview';
-      const url = processPreviewAnnotation(annotation, '/Users/foo/storybook/');
-      expect(url).toBe('storybook-addon/preview');
-    });
+  it('should keep absolute node_modules paths', () => {
+    const annotation = '/Users/foo/storybook/node_modules/storybook-addon/preview';
+    const url = processPreviewAnnotation(annotation, '/Users/foo/storybook/');
+    expect(url).toBe('/Users/foo/storybook/node_modules/storybook-addon/preview');
+  });
 
-    it('should convert relative paths outside the root into absolute', () => {
-      const annotation = '../parent.js';
-      const url = processPreviewAnnotation(annotation, '/Users/foo/storybook/');
-      expect(url).toBe('/Users/foo/parent.js');
-    });
+  it('should convert relative paths outside the root into absolute', () => {
+    const annotation = '../parent.js';
+    const url = processPreviewAnnotation(annotation, '/Users/foo/storybook/');
+    expect(url).toBe('/Users/foo/parent.js');
+  });
 
-    it('should not change absolute paths outside of the project root', () => {
-      const annotation = '/Users/foo/parent.js';
-      const url = processPreviewAnnotation(annotation, '/Users/foo/storybook/');
-      expect(url).toBe(annotation);
-    });
+  it('should not change absolute paths outside of the project root', () => {
+    const annotation = '/Users/foo/parent.js';
+    const url = processPreviewAnnotation(annotation, '/Users/foo/storybook/');
+    expect(url).toBe(annotation);
   });
 
-  onlyWindows(() => {
-    it('should convert absolute windows filesystem paths into urls relative to project root', () => {
-      const annotation = 'C:/foo/storybook/.storybook/preview.js';
-      const url = processPreviewAnnotation(annotation, 'C:/foo/storybook');
-      expect(url).toBe('/.storybook/preview.js');
-    });
-    it('should convert relative paths outside the root into absolute on Windows', () => {
-      const annotation = '../parent.js';
-      const url = processPreviewAnnotation(annotation, 'C:/Users/foo/storybook/');
-      expect(url).toBe('C:/Users/foo/parent.js');
-    });
+  it('should keep absolute windows filesystem paths as is', () => {
+    const annotation = 'C:/foo/storybook/.storybook/preview.js';
+    const url = processPreviewAnnotation(annotation, 'C:/foo/storybook');
+    expect(url).toBe('C:/foo/storybook/.storybook/preview.js');
+  });
+  it('should convert relative paths outside the root into absolute on Windows', () => {
+    const annotation = '../parent.js';
+    const url = processPreviewAnnotation(annotation, 'C:/Users/foo/storybook/');
+    expect(url).toBe('C:/Users/foo/parent.js');
+  });
+
+  it('should not change Windows absolute paths outside of the project root', () => {
+    const annotation = 'D:/Users/foo/parent.js';
+    const url = processPreviewAnnotation(annotation, 'D:/Users/foo/storybook/');
+    expect(url).toBe(annotation);
+  });
+
+  it('should normalize absolute Windows paths using \\', () => {
+    const annotation = 'C:\\foo\\storybook\\.storybook\\preview.js';
+    const url = processPreviewAnnotation(annotation, 'C:\\foo\\storybook');
+    expect(url).toBe('C:/foo/storybook/.storybook/preview.js');
+  });
 
-    it('should not change Windows absolute paths outside of the project root', () => {
-      const annotation = 'D:/Users/foo/parent.js';
-      const url = processPreviewAnnotation(annotation, 'D:/Users/foo/storybook/');
-      expect(url).toBe(annotation);
-    });
+  it('should normalize relative Windows paths using \\', () => {
+    const annotation = '.\\src\\stories\\preview';
+    const url = processPreviewAnnotation(annotation, 'C:\\foo\\storybook');
+    expect(url).toBe('C:/foo/storybook/src/stories/preview');
   });
 });

code/builders/builder-vite/src/utils/process-preview-annotation.ts

@@ -1,54 +1,25 @@
-import { isAbsolute, relative, resolve } from 'node:path';
-
-import { stripAbsNodeModulesPath } from 'storybook/internal/common';
 import type { PreviewAnnotation } from 'storybook/internal/types';
 
-import slash from 'slash';
+import { isAbsolute, normalize, resolve } from 'pathe';
 
-/**
- * Preview annotations can take several forms, and vite needs them to be a bit more restrained.
- *
- * For node_modules, we want bare imports (so vite can process them), and for files in the user's
- * source, we want URLs absolute relative to project root.
- */
-export function processPreviewAnnotation(path: PreviewAnnotation | undefined, projectRoot: string) {
-  // If entry is an object, take the first, which is the
-  // bare (non-absolute) specifier.
+/** Preview annotations can take several forms, so we normalize them here to absolute file paths. */
+export function processPreviewAnnotation(path: PreviewAnnotation, projectRoot: string) {
+  // If entry is an object, take the absolute specifier.
   // This is so that webpack can use an absolute path, and
   // continue supporting super-addons in pnp/pnpm without
   // requiring them to re-export their sub-addons as we do
   // in addon-essentials.
   if (typeof path === 'object') {
-    return path.bare;
-  }
-
-  // This should not occur, since we use `.filter(Boolean)` prior to
-  // calling this function, but this makes typescript happy
-  if (!path) {
-    throw new Error('Could not determine path for previewAnnotation');
+    console.log(
+      'Deprecated: Preview annotations should be strings, not objects. Use the `absolute` property instead.'
+    );
+    return path.absolute;
   }
 
-  // For addon dependencies that use require.resolve(), we need to convert to a bare path
-  // so that vite will process it as a dependency (cjs -> esm, etc).
-  // TODO: Evaluate if searching for node_modules in a yarn pnp environment is correct
-  if (path.includes('node_modules')) {
-    return stripAbsNodeModulesPath(path);
+  // If it's already an absolute path, return it.
+  if (isAbsolute(path)) {
+    return normalize(path);
   }
-
-  // resolve absolute paths relative to project root
-  const relativePath = isAbsolute(path) ? slash(relative(projectRoot, path)) : path;
-
-  // resolve relative paths into absolute urls
-  // note: this only works if vite's projectRoot === cwd.
-  if (relativePath.startsWith('./')) {
-    return slash(relativePath.replace(/^\.\//, '/'));
-  }
-
-  // If something is outside of root, convert to absolute.  Uncommon?
-  if (relativePath.startsWith('../')) {
-    return slash(resolve(projectRoot, relativePath));
-  }
-
-  // At this point, it must be relative to the root but not start with a ./ or ../
-  return slash(`/${relativePath}`);
+  // resolve relative paths, relative to project root
+  return normalize(resolve(projectRoot, path));
 }

code/builders/builder-webpack5/src/preview/virtual-module-mapping.ts

@@ -33,7 +33,6 @@ export const getVirtualModules = async (options: Options) => {
         // If entry is an object, use the absolute import specifier.
         // This is to maintain back-compat with community addons that bundle other addons
         // and package managers that "hide" sub dependencies (e.g. pnpm / yarn pnp)
-        // The vite builder uses the bare import specifier.
         if (typeof entry === 'object') {
           return entry.absolute;
         }