diff --git a/code/.storybook/preview.tsx b/code/.storybook/preview.tsx index 23f95a0c5d5e..7cdfa722090c 100644 --- a/code/.storybook/preview.tsx +++ b/code/.storybook/preview.tsx @@ -117,9 +117,6 @@ const ThemedSetRoot = () => { return null; }; -// eslint-disable-next-line no-underscore-dangle -const preview = (window as any).__STORYBOOK_PREVIEW__ as PreviewWeb | undefined; -const channel = (window as any).__STORYBOOK_ADDONS_CHANNEL__ as Channel | undefined; export const loaders = [ /** * This loader adds a DocsContext to the story, which is required for the most Blocks to work. A @@ -134,6 +131,9 @@ 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 | undefined; + const channel = (window as any).__STORYBOOK_ADDONS_CHANNEL__ as Channel | undefined; // __STORYBOOK_PREVIEW__ and __STORYBOOK_ADDONS_CHANNEL__ is set in the PreviewWeb constructor // which isn't loaded in portable stories/vitest if (!relativeCsfPaths || !preview || !channel) { diff --git a/code/builders/builder-vite/src/codegen-modern-iframe-script.test.ts b/code/builders/builder-vite/src/codegen-modern-iframe-script.test.ts new file mode 100644 index 000000000000..02c1e9fa6c02 Binary files /dev/null and b/code/builders/builder-vite/src/codegen-modern-iframe-script.test.ts differ diff --git a/code/builders/builder-vite/src/codegen-modern-iframe-script.ts b/code/builders/builder-vite/src/codegen-modern-iframe-script.ts index dffb3144d67a..da278c124f83 100644 --- a/code/builders/builder-vite/src/codegen-modern-iframe-script.ts +++ b/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,8 +97,9 @@ export async function generateModernIframeScriptCode(options: Options, projectRo * * @todo Inline variable and remove `noinspection` */ - const code = ` + const code = dedent` import { setup } from 'storybook/internal/preview/runtime'; + import '${SB_VIRTUAL_FILES.VIRTUAL_ADDON_SETUP_FILE}'; setup(); @@ -75,14 +107,17 @@ export async function generateModernIframeScriptCode(options: Options, projectRo import { composeConfigs, PreviewWeb, ClientApi } from 'storybook/internal/preview-api'; 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); +} diff --git a/code/builders/builder-vite/src/optimizeDeps.ts b/code/builders/builder-vite/src/optimizeDeps.ts index 74dd0090be42..8a35d9169983 100644 --- a/code/builders/builder-vite/src/optimizeDeps.ts +++ b/code/builders/builder-vite/src/optimizeDeps.ts @@ -144,6 +144,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 'sb-original/default-loader', 'sb-original/image-context', 'slash', diff --git a/code/builders/builder-vite/src/utils/process-preview-annotation.test.ts b/code/builders/builder-vite/src/utils/process-preview-annotation.test.ts index 3a01623919d5..f4f243ebc439 100644 --- a/code/builders/builder-vite/src/utils/process-preview-annotation.test.ts +++ b/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'); }); }); diff --git a/code/builders/builder-vite/src/utils/process-preview-annotation.ts b/code/builders/builder-vite/src/utils/process-preview-annotation.ts index 7ff6f7a00081..3e4dcf988eba 100644 --- a/code/builders/builder-vite/src/utils/process-preview-annotation.ts +++ b/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)); } diff --git a/code/builders/builder-webpack5/src/preview/virtual-module-mapping.ts b/code/builders/builder-webpack5/src/preview/virtual-module-mapping.ts index ec55e40c5291..e6c9da79c3cc 100644 --- a/code/builders/builder-webpack5/src/preview/virtual-module-mapping.ts +++ b/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; }