Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions scripts/bench/PERF-METHODOLOGY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
47 changes: 47 additions & 0 deletions scripts/bench/docgen-shared/react-renderer-module.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
74 changes: 56 additions & 18 deletions scripts/bench/docgen-shared/react-renderer-module.ts
Original file line number Diff line number Diff line change
@@ -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<T>(relativePath: string): Promise<T> {
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 {
Expand Down
2 changes: 2 additions & 0 deletions scripts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
Loading