diff --git a/scripts/bench/PERF-METHODOLOGY.md b/scripts/bench/PERF-METHODOLOGY.md index a62bf89d265c..7b4ab814b4d7 100644 --- a/scripts/bench/PERF-METHODOLOGY.md +++ b/scripts/bench/PERF-METHODOLOGY.md @@ -115,6 +115,7 @@ An engine whose package does not resolve is reported as skipped with a reason ra This works only for an engine whose child imports the versioned package directly, the way `engines/vue-component-meta.ts` does. Where the harness reaches the engine through repo source instead - `react-legacy` goes via `loadReactRendererModule` into `code/renderers/react`, which imports `react-docgen` by bare specifier - no child flag can redirect that import, and a version pair needs a different approach entirely. +Declaring `@storybook/react` as a dependency of `scripts` is what makes that source reachable in the first place, but it does not make the specifier redirectable: pointing the renderer at a second `react-docgen` would take a module resolution hook registered in the child, which is not built. Four data edits: diff --git a/scripts/bench/docgen-shared/react-renderer-module.test.ts b/scripts/bench/docgen-shared/react-renderer-module.test.ts new file mode 100644 index 000000000000..8792fec4dab1 --- /dev/null +++ b/scripts/bench/docgen-shared/react-renderer-module.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest'; + +import { loadReactRendererModule, rendererModuleError } from './react-renderer-module.ts'; + +/** + * The message below is the reason this module exists rather than a hardcoded path: without the + * redirect, a missing `code/core` build surfaces as a resolution failure naming a file no harness + * ever mentions, and the reader has no way to get from it to `yarn nx compile core`. + */ +const missingCoreBuild = new Error( + "Cannot find module '/repo/node_modules/storybook/dist/common/index.js' imported from " + + '/repo/code/renderers/react/src/componentManifest/utils.ts' +); + +describe('rendererModuleError', () => { + it('turns a missing core build into the command that fixes it', () => { + const error = rendererModuleError('utils.ts', missingCoreBuild); + expect(error.message).toContain('yarn nx compile core'); + expect(error.message).toContain('utils.ts'); + }); + + it('keeps the original resolution failure as the cause', () => { + // The rewritten message replaces the one Node produced, so the path it named has to survive or + // a genuinely different resolution problem becomes undebuggable. + expect(rendererModuleError('utils.ts', missingCoreBuild).message).toContain( + 'storybook/dist/common/index.js' + ); + }); + + it('passes through failures that are not about the core build', () => { + const unrelated = new Error('Unexpected token'); + expect(rendererModuleError('utils.ts', unrelated)).toBe(unrelated); + }); + + it('handles a thrown non-Error without losing it', () => { + expect(rendererModuleError('utils.ts', 'boom').message).toBe('boom'); + }); +}); + +describe('loadReactRendererModule', () => { + it('resolves the renderer source through the declared dependency', async () => { + // Reaching a real module is what proves the package resolution works; a wrong anchor would fail + // here rather than in whichever harness ran first. + const mod = await loadReactRendererModule<{ invalidateCache: () => void }>('utils.ts'); + expect(typeof mod.invalidateCache).toBe('function'); + }); +}); diff --git a/scripts/bench/docgen-shared/react-renderer-module.ts b/scripts/bench/docgen-shared/react-renderer-module.ts index 059be30e6ae4..4d81b39f21a4 100644 --- a/scripts/bench/docgen-shared/react-renderer-module.ts +++ b/scripts/bench/docgen-shared/react-renderer-module.ts @@ -1,28 +1,66 @@ -import { join } from 'node:path'; +import { existsSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { dirname, join } from 'node:path'; import { pathToFileURL } from 'node:url'; /** - * The React harnesses measure the renderer's own extraction code, so they load it from source - * rather than through the package's published surface. + * The React harnesses measure the renderer's own extraction code, so they load modules out of + * `@storybook/react`'s source rather than its published surface - `componentManifest/` is not in the + * exports map, and `files` excludes `src/**` outright. * - * Joined as a path rather than concatenated into a URL: string concatenation only happens to work - * while the directory ends in a separator and the caller's argument does not begin with one. + * That reach is deliberate, but it anchors on the resolved package instead of a path relative to + * this file. `scripts` declares `@storybook/react`, so the coupling is recorded in the workspace + * graph rather than living only in a `../../../` that nothing checks, and either tree can move + * without the failure surfacing as a missing file inside an unrelated import. */ -const COMPONENT_MANIFEST = join( - import.meta.dirname, - '..', - '..', - '..', - 'code', - 'renderers', - 'react', - 'src', - 'componentManifest' -); +const require = createRequire(import.meta.url); + +let cachedDir: string | undefined; + +function componentManifestDir(): string { + if (cachedDir) { + return cachedDir; + } + const packageRoot = dirname(require.resolve('@storybook/react/package.json')); + const dir = join(packageRoot, 'src', 'componentManifest'); + if (!existsSync(dir)) { + throw new Error( + `@storybook/react resolved to ${packageRoot}, which has no src/componentManifest. The React ` + + `harnesses read the renderer's source, so they need the workspace checkout rather than a ` + + `published copy.` + ); + } + cachedDir = dir; + return dir; +} export async function loadReactRendererModule(relativePath: string): Promise { - const url = pathToFileURL(join(COMPONENT_MANIFEST, relativePath)).href; - return (await import(url)) as T; + const url = pathToFileURL(join(componentManifestDir(), relativePath)).href; + try { + return (await import(url)) as T; + } catch (err) { + throw rendererModuleError(relativePath, err); + } +} + +/** + * The renderer's source imports `storybook/internal/*`, which resolves to `code/core`'s build + * output. When that is missing, Node names a path the harness never mentioned, so the one thing the + * reader needs - compile core first - is what the message leaves out. + * + * Exported because that redirect is the point of this module rather than an implementation detail: + * if the match stops recognising the failure, the harness goes back to reporting the unhelpful + * error and nothing else would notice. + */ +export function rendererModuleError(relativePath: string, err: unknown): Error { + const message = err instanceof Error ? err.message : String(err); + if (/storybook[/\\]dist[/\\]/.test(message)) { + return new Error( + `loading ${relativePath} from the React renderer needs storybook's build output, which is ` + + `missing. Run \`yarn nx compile core\` and try again.\n cause: ${message}` + ); + } + return err instanceof Error ? err : new Error(message); } export interface ComponentRefLike { diff --git a/scripts/package.json b/scripts/package.json index 0d18f6a4e1b6..7169081cf2aa 100644 --- a/scripts/package.json +++ b/scripts/package.json @@ -57,6 +57,7 @@ "@octokit/request": "^8.4.1", "@openai/codex-sdk": "^0.117.0", "@polka/parse": "^1.0.0-next.28", + "@storybook/react": "workspace:*", "@testing-library/dom": "^10.4.0", "@testing-library/jest-dom": "6.9.1", "@testing-library/react": "^16.0.0", @@ -145,6 +146,7 @@ "simple-git": "^3.30.0", "slash": "^3.0.0", "sort-package-json": "^3.5.0", + "storybook": "workspace:*", "tiny-invariant": "^1.3.3", "tinyexec": "^0.3.0", "tinyglobby": "^0.2.15", diff --git a/yarn.lock b/yarn.lock index cde6482d35e8..25de2481c5ab 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12765,6 +12765,7 @@ __metadata: "@octokit/request": "npm:^8.4.1" "@openai/codex-sdk": "npm:^0.117.0" "@polka/parse": "npm:^1.0.0-next.28" + "@storybook/react": "workspace:*" "@testing-library/dom": "npm:^10.4.0" "@testing-library/jest-dom": "npm:6.9.1" "@testing-library/react": "npm:^16.0.0" @@ -12855,6 +12856,7 @@ __metadata: simple-git: "npm:^3.30.0" slash: "npm:^3.0.0" sort-package-json: "npm:^3.5.0" + storybook: "workspace:*" tiny-invariant: "npm:^1.3.3" tinyexec: "npm:^0.3.0" tinyglobby: "npm:^0.2.15"