fix(checklist): respect disableTelemetry config in initializeChecklist#34435
fix(checklist): respect disableTelemetry config in initializeChecklist#34435mixelburg wants to merge 82 commits into
Conversation
…test-mocker-resolution Builder-Webpack5: Fix @vitest/mocker resolution issue (cherry picked from commit abd4b01)
…-init-telemetry CLI: Add init telemetry for CLI integrations (cherry picked from commit 94f79bf)
…rom-10.2.0 Release: Patch 10.2.1
…ent-agent-detection-telemetry Telemetry: Add agent detection (cherry picked from commit 945a876)
…-factories-monorepo Docs: Add FAQ about sharing config in monorepos for CSF Next (cherry picked from commit 90b7abb)
…-loginurl Composition: Handle 401 responses with loginUrl from Chromatic (cherry picked from commit afdf02a)
…vitest-prevent-double-nesting Addon-Vitest: Append Storybook project to existing test.projects array without double nesting (cherry picked from commit 5bbb915)
…vitest-support-simple-workspace-config Addon Vitest: Support simple vite.config without defineConfig helper (cherry picked from commit 751f18f)
…ckage-benchmarking Build: Fix package benchmarking (cherry picked from commit 48cecda)
…st-flake E2E: Fix flakey test (cherry picked from commit ac039aa)
…vitest-requireassertions Addon-Vitest: Update Vitest plugin configuration to disable requireAssertions for expect (cherry picked from commit b18d12e)
…rom-10.2.1 Release: Patch 10.2.2
Core: Fix `previewHref` when current path does not end with a slash (cherry picked from commit 00f506c)
…s-addon-vitest-31768 Addon-Vitest: Normalize Windows paths in addon-vitest automigration (cherry picked from commit 3802b16)
…rom-10.2.2 Release: Patch 10.2.3
…mod-windows Codemod: Fix glob pattern handling on Windows (cherry picked from commit 4d07f57)
…ories-non-interactive-mode CSFFactories: Add non-interactive mode and --glob flag (cherry picked from commit e9c8a87)
…le-select Globals: Repair dynamicTitle: false for user-defined tools (cherry picked from commit d355c8b)
…s-cross-file-imports CSF: Fix cross-file story imports in csf-factories codemod (cherry picked from commit 8ef9070)
…lag-works Logger: honor --loglevel for npmlog output (cherry picked from commit 02fdf2a)
…rom-10.2.6 Release: Patch 10.2.7
…o-telemetry-patch-label Telemetry: Add Expo metaframework (cherry picked from commit 5175dce)
…hannel-events Telemetry: Add share events (cherry picked from commit ce42f59)
…it-telemetry Telemetry: Add init exit event (cherry picked from commit 5e1084c)
…ent-view-undefined Test: Update event creation logic in user-event package (cherry picked from commit a4a0dff)
…rom-10.2.7 Release: Patch 10.2.8
…-mocker-relative-path Builder-Vite: Use relative path for mocker entry in production builds (cherry picked from commit 3c49657)
…c-issue Builder-Vite: Update dependencies react-vite framework (cherry picked from commit dd088b3)
…k-component-override Next.js: Fix Link component override in appDirectory configuration (cherry picked from commit 40e556a)
…e-config-file-detection Addon-Vitest: Improve config file detection in monorepos (cherry picked from commit d7c87fa)
…rom-10.2.8 Release: Patch 10.2.9
📝 WalkthroughWalkthroughVersion bump to 10.2.9 across Storybook monorepo with functional enhancements: Vitest addon improvements (config discovery, monorepo support), mocking utilities refactoring (extracting findMockRedirect, replacing runtime bundling), telemetry enhancements (agent detection, share events), CLI automigrations (csf-factories glob/yes support), and framework/builder updates (Next.js link mock, Vite output directory enforcement). Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
✨ Finishing Touches⚔️ Resolve merge conflicts
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
code/core/src/common/utils/get-storybook-refs.ts (1)
61-81:⚠️ Potential issue | 🟠 MajorHandle second-fetch failures in
checkRefto avoid promise rejection leakage.
checkRefcurrently converts first-fetch failures tofalse, but if the second fetch fails (the JSON probe), the promise rejects instead of returningfalse. That can break callers that expect a boolean result.🔧 Proposed fix
-export const checkRef = (url: string) => - fetch(`${url}/iframe.html`).then( - async ({ ok, status }) => { - if (ok) { - if (status !== 200) { - return false; - } - - // so the status is ok, but if we'd ask for JSON we might get a response saying we need to authenticate. - const data = await fetch(`${url}/iframe.html`, { - headers: { Accept: 'application/json' }, - }); - // we might receive non-JSON as a response, because the service ignored our request for JSON response type. - if (data.ok && (await (data as any).json().catch(() => ({}))).loginUrl) { - return false; - } - } - return ok; - }, - () => false - ); +export const checkRef = async (url: string) => { + try { + const { ok, status } = await fetch(`${url}/iframe.html`); + if (!ok || status !== 200) { + return false; + } + + // so the status is ok, but if we'd ask for JSON we might get a response saying we need to authenticate. + const data = await fetch(`${url}/iframe.html`, { + headers: { Accept: 'application/json' }, + }); + // we might receive non-JSON as a response, because the service ignored our request for JSON response type. + if (data.ok && (await (data as any).json().catch(() => ({}))).loginUrl) { + return false; + } + return true; + } catch { + return false; + } +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/common/utils/get-storybook-refs.ts` around lines 61 - 81, checkRef can reject if the second fetch (the JSON probe to `${url}/iframe.html`) throws; update checkRef so any errors from the second fetch or from parsing JSON are caught and result in returning false rather than propagating a rejection. Specifically, inside the success branch of export const checkRef, wrap the second fetch and the subsequent (data as any).json() call in a try/catch (or append a .catch) so that failures during the JSON probe cause the function to return false; reference symbols: checkRef, the second fetch to `${url}/iframe.html`, and the (data as any).json() call.code/frameworks/vue3-vite/package.json (1)
69-69:⚠️ Potential issue | 🔴 CriticalFix invalid
gitHeadacross all 39 package.json files in the monorepo.The
gitHeadfield valuea8e7fd8a655c69780bc20b9749d2699e45beae1lis malformed—it ends with lowercaselinstead of a valid hexadecimal character. Git commit SHAs must be 40-character hexadecimal strings (0-9, a-f only). This issue affects all package.json files in the monorepo and will break package publishing. The current HEAD commit SHA ise22f6a06600cbc0a99768604c2f94f822ffd16d2; verify the correct commit SHA and update all occurrences accordingly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/frameworks/vue3-vite/package.json` at line 69, The gitHead field in package.json files contains a malformed SHA ("a8e7fd8a655c69780bc20b9749d2699e45beae1l"); locate every package.json (all 39) and replace the invalid gitHead value with the correct 40‑char hexadecimal commit SHA "e22f6a06600cbc0a99768604c2f94f822ffd16d2" (ensure the value is a plain hex string, no trailing letters or quotes changes), or programmatically run a safe search-and-replace for the gitHead property key to update the value consistently across the monorepo and then verify by running a quick JSON parse/lint of each package.json to confirm validity.
♻️ Duplicate comments (7)
code/lib/cli-sb/package.json (1)
34-34:⚠️ Potential issue | 🔴 CriticalDuplicate: Invalid
gitHeadvalue.Same malformed
gitHeadas flagged incode/frameworks/vue3-vite/package.json:69.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/cli-sb/package.json` at line 34, The package.json contains a malformed gitHead value ("gitHead": "a8e7fd8a655c69780bc20b9749d2699e45beae1l"); update the gitHead field to a valid 40-character hexadecimal commit hash (remove the trailing non-hex 'l') or remove the gitHead entry entirely; target the "gitHead" key in package.json to make this change so it matches the valid gitHead in the other package.json.code/renderers/svelte/package.json (1)
73-73:⚠️ Potential issue | 🔴 CriticalDuplicate: Invalid
gitHeadvalue.Same malformed
gitHeadas flagged incode/frameworks/vue3-vite/package.json:69. This must be corrected to a valid 40-character hexadecimal SHA before publishing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/renderers/svelte/package.json` at line 73, The package.json contains an invalid gitHead value under the "gitHead" field (currently "a8e7fd8a655c69780bc20b9749d2699e45beae1l"); replace this malformed value with the correct 40-character hexadecimal commit SHA for the release (ensure only [0-9a-f] characters and length 40) so the "gitHead" entry in package.json is a valid commit SHA before publishing.code/lib/eslint-plugin/package.json (1)
70-70:⚠️ Potential issue | 🔴 CriticalDuplicate: Invalid
gitHeadvalue.Same malformed
gitHeadas flagged incode/frameworks/vue3-vite/package.json:69.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/eslint-plugin/package.json` at line 70, The gitHead field contains an invalid value ("a8e7fd8a655c69780bc20b9749d2699e45beae1l") — remove or replace the trailing non-hex character so it is a valid 40-character SHA, or delete the gitHead property if you don't have the exact commit; update the "gitHead" key in package.json accordingly (same fix as applied to the other package.json with the duplicate malformed gitHead).code/lib/react-dom-shim/package.json (1)
51-51:⚠️ Potential issue | 🔴 CriticalDuplicate: Invalid
gitHeadvalue.Same malformed
gitHeadas flagged incode/frameworks/vue3-vite/package.json:69.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/react-dom-shim/package.json` at line 51, The package.json has an invalid gitHead value "a8e7fd8a655c69780bc20b9749d2699e45beae1l" — the trailing non-hex character makes the commit hash malformed; update the gitHead field in code/lib/react-dom-shim/package.json (the "gitHead" property) to a valid commit SHA (remove the trailing 'l' or replace with the correct hex hash) so it matches the proper 40-character hex format used elsewhere.code/renderers/vue3/package.json (1)
68-68:⚠️ Potential issue | 🔴 CriticalDuplicate: Invalid
gitHeadvalue.Same malformed
gitHeadas flagged incode/frameworks/vue3-vite/package.json:69.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/renderers/vue3/package.json` at line 68, The package.json contains a malformed gitHead value ("gitHead" key in the file) — replace the invalid entry "a8e7fd8a655c69780bc20b9749d2699e45beae1l" with either a valid 40-character git commit SHA or remove the "gitHead" field entirely; ensure the corrected value matches the repository commit hash format (hex digits only) and mirrors the fix applied to the duplicate in the other package.json.code/frameworks/html-vite/package.json (1)
62-62:⚠️ Potential issue | 🔴 CriticalDuplicate: Invalid
gitHeadvalue.Same malformed
gitHeadas flagged incode/frameworks/vue3-vite/package.json:69.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/frameworks/html-vite/package.json` at line 62, The package.json contains an invalid gitHead value ("gitHead": "a8e7fd8a655c69780bc20b9749d2699e45beae1l") — replace the malformed entry under the "gitHead" key with the correct 40-character hexadecimal commit SHA for this package (or remove the gitHead field if it was added accidentally and regenerate the package metadata), ensuring the value uses only 0-9 and a-f characters; locate the "gitHead" key in this package.json to make the change.code/renderers/web-components/package.json (1)
70-70:⚠️ Potential issue | 🔴 CriticalDuplicate: Invalid
gitHeadvalue.Same malformed
gitHeadas flagged incode/frameworks/vue3-vite/package.json:69.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/renderers/web-components/package.json` at line 70, The package.json contains a malformed gitHead value ("gitHead": "a8e7fd8a655c69780bc20b9749d2699e45beae1l"); locate the gitHead property and fix it by replacing the trailing non-hex character with the correct commit hash (ensure it is a valid lowercase hex SHA) or remove the gitHead field if not needed; use the same corrected value used in the other package.json (or obtain the proper commit id from your git history) so both package.json files match.
🧹 Nitpick comments (22)
code/core/src/manager/components/preview/Viewport.tsx (1)
14-15: Pre-existing: relative imports missing explicit file extensions.As per coding guidelines, TypeScript imports should use explicit file extensions (e.g.,
./Iframe.tsx,./NumericInput.tsx). These imports predate this PR's change, so no action required here, but consider addressing in a follow-up.Suggested fix
-import { IFrame } from './Iframe'; -import { NumericInput } from './NumericInput'; +import { IFrame } from './Iframe.tsx'; +import { NumericInput } from './NumericInput.tsx';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/manager/components/preview/Viewport.tsx` around lines 14 - 15, Update the two relative imports in Viewport.tsx to include explicit TypeScript file extensions: change the import specifiers for the IFrame and NumericInput modules from './Iframe' and './NumericInput' to './Iframe.tsx' and './NumericInput.tsx' respectively; ensure the import statements that reference the IFrame and NumericInput symbols in the Viewport.tsx component are updated so the module resolver sees the explicit .tsx extensions.code/core/src/components/components/Select/Select.stories.tsx (1)
1490-1493: Tighten these assertions to avoid false positives.These checks currently validate substrings only, so they can pass even if selected and default labels are both rendered. Please add explicit negative checks (e.g., selected title not present when
showSelectedOptionTitle=false) or exact-match assertions.Suggested assertion hardening
export const ShowSelectedOptionTitleFalse = meta.story({ @@ play: async ({ canvas, step }) => { await step('Verify default title is shown instead of selected option', async () => { const selectButton = await canvas.findByRole('button'); expect(selectButton).toHaveTextContent('Animal'); + expect(selectButton).not.toHaveTextContent('Frog'); }); }, });Also applies to: 1504-1507, 1519-1522, 1534-1537
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/components/components/Select/Select.stories.tsx` around lines 1490 - 1493, The assertion in the Storybook tests uses toHaveTextContent which allows substring matches and can give false positives (e.g., both default and selected labels present); update the assertions in the Select.stories tests that use the 'selectButton' lookup inside the step blocks (e.g., the steps titled "Verify selected option title is shown" and the related steps at the other occurrences) to use exact-match checks or add negative checks: replace substring assertions with an equality/assertExact match against the full button text, or add expect(selectButton).not.toHaveTextContent('<default label>') / expect(selectButton).toHaveTextContent('<selected label>') depending on the case, and when testing showSelectedOptionTitle=false assert the selected title is absent; apply the same change to the other indicated steps (the ones around the later step blocks).code/core/src/common/utils/get-storybook-refs.test.ts (1)
31-34: Add a test for “second fetch fails after initial 200”.Current tests cover initial fetch rejection, but not failure of the JSON probe request. Adding that case would prevent regressions in the boolean contract of
checkRef.🧪 Suggested test
it('returns false when fetch fails', async () => { vi.spyOn(global, 'fetch').mockRejectedValue(new Error('Network error')); expect(await checkRef('https://chromatic.com')).toBe(false); }); + + it('returns false when auth probe fetch fails after initial 200', async () => { + vi.spyOn(global, 'fetch') + .mockResolvedValueOnce({ ok: true, status: 200, json: async () => ({}) } as Response) + .mockRejectedValueOnce(new Error('Network error')); + expect(await checkRef('https://chromatic.com')).toBe(false); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/common/utils/get-storybook-refs.test.ts` around lines 31 - 34, Add a new unit test for checkRef that simulates the first fetch resolving with a 200 Response and the subsequent JSON probe fetch rejecting; specifically, mock global.fetch so the first call returns a successful Response-like object and the second call rejects with an error, then assert checkRef('https://chromatic.com') resolves to false to ensure the boolean contract is preserved when the probe fails.code/lib/codemod/src/index.test.ts (1)
7-7: Missing explicit file extension in relative import.Per coding guidelines, TypeScript files should use explicit file extensions for relative imports.
Proposed fix
-import { runCodemod } from './index'; +import { runCodemod } from './index.ts';As per coding guidelines:
**/*.{ts,tsx}: Use explicit file extensions for relative code imports and exports in TypeScript🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/codemod/src/index.test.ts` at line 7, Update the relative import to include an explicit TypeScript extension: change the import of runCodemod in index.test.ts (currently "import { runCodemod } from './index';") to use "./index.ts" so the test follows the rule requiring explicit .ts/.tsx extensions for relative imports.code/lib/codemod/src/index.ts (1)
12-12: Missing explicit file extension in relative import.Per coding guidelines, TypeScript files should use explicit file extensions for relative imports.
Proposed fix
-import { jscodeshiftToPrettierParser } from './lib/utils'; +import { jscodeshiftToPrettierParser } from './lib/utils.ts';As per coding guidelines:
**/*.{ts,tsx}: Use explicit file extensions for relative code imports and exports in TypeScript (e.g.,./foo.tsor./bar.tsx)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/codemod/src/index.ts` at line 12, The import in index.ts uses a relative path without an explicit TypeScript extension; update the import statement that references jscodeshiftToPrettierParser to use an explicit .ts extension (change './lib/utils' to './lib/utils.ts') so the module import follows the TypeScript explicit-extension guideline and resolves correctly.docs/get-started/frameworks/angular.mdx (1)
292-292: Tighten wording for readability in the loglevel description.Small wording tweak to make the sentence read more naturally.
✏️ Suggested docs tweak
-| `"loglevel"` | Controls level of logging during build. Can be one of: [trace, debug, info (default), warn, error, silent]. <br /> `"loglevel": "info"` | +| `"loglevel"` | Controls the logging level during build. It can be one of: [trace, debug, info (default), warn, error, silent]. <br /> `"loglevel": "info"` |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/get-started/frameworks/angular.mdx` at line 292, Update the table entry for "loglevel" to a tighter, more natural sentence: replace the current text with something like: Controls the logging level during the build; one of: trace, debug, info (default), warn, error, silent. Example: "loglevel": "info". Target the table cell containing the "loglevel" description so the wording change applies to that exact entry.code/core/package.json (1)
129-129: Consider documenting why this export lacks atypesfield.Unlike other exports in this file that include both
typesanddefaultfields, this newmocker-runtimeexport only specifies adefaultpath. Based on the relatedbuild-config.tschange, this appears intentional (dts: false), but a brief comment explaining the rationale would help future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/package.json` at line 129, The package.json export for "./internal/mocking-utils/mocker-runtime" only provides a "default" path and omits a "types" entry; update the file to add a short inline comment next to the export explaining this is intentional because the build config (see build-config.ts) sets dts: false for the mocker-runtime build, so no generated .d.ts exists, or alternatively add a corresponding "types" mapping if you want type declarations—reference the export key "mocker-runtime" and the build-config.ts setting "dts: false" when making the change.code/lib/cli-storybook/src/codemod/helpers/config-to-csf-factory.ts (1)
10-10: Add explicit.tsextension to the import path.As per coding guidelines, TypeScript imports should use explicit file extensions.
Proposed fix
import { - addImportToTop, + addImportToTop, cleanupTypeImports, getConfigProperties, removeExportDeclarations, -} from './csf-factories-utils'; +} from './csf-factories-utils.ts';As per coding guidelines:
**/*.{ts,tsx}: Use explicit file extensions for relative code imports and exports in TypeScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/cli-storybook/src/codemod/helpers/config-to-csf-factory.ts` at line 10, Update the import that brings in addImportToTop so the module specifier uses an explicit TypeScript extension (e.g., change the relative import from ".../someModule" to ".../someModule.ts"); locate the import list that includes addImportToTop in config-to-csf-factory and append ".ts" to the referenced file path to comply with the TypeScript explicit-extension rule.scripts/build/utils/generate-bundle.ts (1)
10-10: Minor formatting inconsistency.The import lacks spaces around the braces, inconsistent with other imports in this file.
🔧 Suggested fix for consistent formatting
-import {raw as rawPlugin} from 'esbuild-raw-plugin'; +import { raw as rawPlugin } from 'esbuild-raw-plugin';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build/utils/generate-bundle.ts` at line 10, The import statement "import {raw as rawPlugin} from 'esbuild-raw-plugin';" is formatted inconsistently; update it to include spaces around the braces and the "as" alias to match the file's import style (e.g., "import { raw as rawPlugin } from 'esbuild-raw-plugin';"), locating and changing the line that imports rawPlugin from esbuild-raw-plugin in generate-bundle.ts.code/core/src/common/utils/resolve-path-in-sb-cache.test.ts (2)
10-18: Consider usingspy: trueoption for mocks.As per coding guidelines,
vi.mock()should use thespy: trueoption for package and file mocks. This enables spying on the mocked functions while still allowing custom implementations.♻️ Suggested refactor
-vi.mock('empathic/package', () => ({ - cache: vi.fn(), -})); +vi.mock('empathic/package', { spy: true }); -vi.mock('../versions', () => ({ - default: { - storybook: '10.3.0-alpha.1', - }, -})); +vi.mock('../versions', { spy: true });Then set up the default mock implementations in
beforeEach:beforeEach(() => { vi.clearAllMocks(); vi.mocked(versions).storybook = '10.3.0-alpha.1'; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/common/utils/resolve-path-in-sb-cache.test.ts` around lines 10 - 18, Update the test mocks to use the spy option and initialize values in beforeEach: change the static vi.mock calls for 'empathic/package' and '../versions' to include { spy: true } so the mocked functions can be spied on, and move the default implementation of versions.storybook into a beforeEach where you call vi.clearAllMocks() and set vi.mocked(versions).storybook = '10.3.0-alpha.1'; ensure you still provide the custom implementation for empathic/package.cache via the spy-enabled mock.
92-106: Reset mock state in a cleaner way.Directly mutating
vi.mocked(versions).storybookand manually resetting it at the end of the test is fragile. If the test fails before the reset, subsequent tests could be affected. Consider usingbeforeEachto set up a consistent state, or use a more explicit mock reset pattern.♻️ Alternative approach using afterEach
+ afterEach(() => { + // Ensure versions mock is reset to default + vi.mocked(versions).storybook = '10.3.0-alpha.1'; + }); + it('should use "unknown" as version when storybook version is not available', () => { const mockCacheDir = '/mock/node_modules/.cache/storybook'; vi.mocked(pkg.cache).mockReturnValue(mockCacheDir); // Mock the versions module to return a falsy value vi.mocked(versions).storybook = '' as any; const result = resolvePathInStorybookCache('test-file', 'test-sub'); expect(result).toContain('unknown'); expect(result).toBe(join(mockCacheDir, 'unknown', 'test-sub', 'test-file')); - - // Reset the mock - vi.mocked(versions).storybook = '10.3.0-alpha.1'; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/common/utils/resolve-path-in-sb-cache.test.ts` around lines 92 - 106, The test mutates versions.storybook directly which is brittle; instead, mock and restore it properly around the test: replace the direct assignment to vi.mocked(versions).storybook with a scoped mock (e.g., vi.spyOn(versions, 'storybook', 'get').mockReturnValueOnce('') or vi.mocked(versions).storybook = '' using vi.fn/mockReturnValueOnce) and add an afterEach or beforeEach that calls vi.restoreAllMocks() or vi.resetAllMocks() so resolvePathInStorybookCache('test-file','test-sub') runs with a controlled mock and the original versions value is automatically restored; reference the versions symbol and the resolvePathInStorybookCache function to locate where to apply the spy/reset.code/builders/builder-vite/src/plugins/vite-inject-mocker/plugin.test.ts (1)
15-23: Use the repo-standard Vitest mock shape in this new file.Because this file is new, I’d switch the
node:urlmock tovi.mock(..., { spy: true })and drivefileURLToPaththroughvi.mocked(...)instead of an inline factory mock. That keeps it aligned with the suite’s mocking conventions while still letting you set the import-time behavior before loading the plugin.As per coding guidelines, "Use
vi.mock()with thespy: trueoption for all package and file mocks in Vitest tests" and "Usevi.mocked()to type and access the mocked functions in Vitest tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/builders/builder-vite/src/plugins/vite-inject-mocker/plugin.test.ts` around lines 15 - 23, Replace the inline factory mock for 'node:url' with the repo-standard spy-style mock: call vi.mock('node:url', { spy: true }) and then obtain a typed handle using vi.mocked(fileURLToPath) to set its return value; also keep the import.meta.resolve stub via vi.stubGlobal but ensure it is set before dynamically importing './plugin.js' so viteInjectMockerRuntime loads with the mocked behavior. Use vi.mocked(fileURLToPath) to configure the mock return (instead of the inline factory) and then await import('./plugin.js') to retrieve viteInjectMockerRuntime.code/lib/cli-storybook/src/codemod/helpers/config-to-csf-factory.test.ts (1)
372-408: Add the named-reexport-only preview case to this fallback coverage.These new assertions cover empty and side-effect-only previews, but the same “missing default export” branch also needs a regression for preview files that only re-export named symbols (for example
export { withTheme } from './decorators'). A small snapshot here would lock down that repo-specific edge case too.Based on learnings, preview files with only named exports (including re-exports) still require a generated
export default definePreview({})because story files import the preview default.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/cli-storybook/src/codemod/helpers/config-to-csf-factory.test.ts` around lines 372 - 408, Add a test case covering the "named re-export only" preview scenario in the same suite: call the existing transform helper with a preview string like "export { withTheme } from './decorators'" and assert it resolves to a snapshot that imports definePreview from '@storybook/react-vite' and emits "export default definePreview({});". Update the test file codemod/helpers/config-to-csf-factory.test.ts by adding an it(...) similar to the existing empty/side-effect tests, referencing the transform helper so the regression for preview files that only re-export named symbols is locked down.code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts (2)
56-63: Mocks cleared after initialization creates potential timing issues.The command is instantiated at lines 56-61, but
vi.clearAllMocks()is called at line 63 after the command creation. This clears any mock calls that may have occurred during construction. If this is intentional, consider adding a comment to clarify.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts` around lines 56 - 63, The test instantiates AddonConfigurationCommand while vi.clearAllMocks() is called afterwards, which can accidentally wipe out mock call history produced during construction; move the vi.clearAllMocks() call to before the new AddonConfigurationCommand(...) creation (or if you intentionally want to drop constructor calls, add an explicit comment explaining that) so that mockPackageManager, mockAddonVitestService, and mockTelemetryService call history is preserved or clearly documented.
17-19: Consider usingspy: truefor mock consistency.The mock for
postinstallAddonuses a factory function instead of{ spy: true }. While this works, it's inconsistent with the other mocks in this file and deviates from the coding guidelines.As per coding guidelines: "Use
vi.mock()with thespy: trueoption for all package and file mocks in Vitest tests."♻️ Suggested refactor
-vi.mock('../../../cli-storybook/src/postinstallAddon', () => ({ - postinstallAddon: vi.fn().mockResolvedValue(undefined), -})); +vi.mock('../../../cli-storybook/src/postinstallAddon', { spy: true });Then in
beforeEach:const { postinstallAddon } = await import('../../../cli-storybook/src/postinstallAddon'); vi.mocked(postinstallAddon).mockResolvedValue(undefined);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts` around lines 17 - 19, The current mock for postinstallAddon is created with a factory function which is inconsistent with the repo guideline—replace the inline factory mock with vi.mock('../../../cli-storybook/src/postinstallAddon', { spy: true }) and then in the test setup (e.g., in beforeEach) import { postinstallAddon } from '../../../cli-storybook/src/postinstallAddon' and call vi.mocked(postinstallAddon).mockResolvedValue(undefined) so the mock uses Vitest's spy mode and matches the other mocks in AddonConfigurationCommand.test.ts.code/addons/vitest/src/postinstall.ts (1)
165-165: Minor: redundant double negation.
options.skipInstallis already a boolean (from PostinstallOptions), so!!options.skipInstallis redundant. However, this is harmless and may be intentional for explicitness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/addons/vitest/src/postinstall.ts` at line 165, The property assignment uses a redundant double negation: change useRemotePkg: !!options.skipInstall to useRemotePkg: options.skipInstall; locate the object where useRemotePkg is set (in postinstall.ts) and remove the extra !! so it directly uses the boolean from PostinstallOptions (options.skipInstall) to keep the code simpler and equivalent.code/lib/cli-storybook/src/codemod/helpers/story-to-csf-factory.ts (1)
7-9: Missing file extensions in relative imports.The coding guidelines specify using explicit file extensions for relative imports in TypeScript files.
As per coding guidelines: "Use explicit file extensions for relative code imports and exports in TypeScript (e.g.,
./foo.tsor./bar.tsx)".♻️ Suggested fix
import type { FileInfo } from '../../automigrate/codemod'; -import { addImportToTop, cleanupTypeImports } from './csf-factories-utils'; -import { removeUnusedTypes } from './remove-unused-types'; +import { addImportToTop, cleanupTypeImports } from './csf-factories-utils.ts'; +import { removeUnusedTypes } from './remove-unused-types.ts';Note: The
typeimport from../../automigrate/codemodmay also need an extension, depending on how it's structured.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/cli-storybook/src/codemod/helpers/story-to-csf-factory.ts` around lines 7 - 9, The relative imports in story-to-csf-factory.ts are missing explicit file extensions; update the import statements for FileInfo (from '../../automigrate/codemod') and the local helpers (addImportToTop, cleanupTypeImports from './csf-factories-utils' and removeUnusedTypes from './remove-unused-types') to include the correct TypeScript file extensions (e.g., .ts or .tsx) so they follow the project's guideline for explicit relative import extensions; ensure you pick the actual extension used by each target module and adjust the import paths accordingly.code/addons/vitest/src/node/vitest-manager.ts (1)
102-112: Consider simplifying the file existence check.The
find.anycall at line 104 withlast: getProjectRoot()is potentially redundant since you're already iterating through specific locations. A directexistsSync(join(location, file))would be more efficient.♻️ Suggested optimization
for (const location of potentialConfigFileLocations) { for (const file of configFiles) { - const maybe = find.any([file], { cwd: location, last: getProjectRoot() }); - if (maybe && existsSync(maybe)) { + const maybe = join(location, file); + if (existsSync(maybe)) { firstVitestConfig ??= maybe; const content = readFileSync(maybe, 'utf8');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/addons/vitest/src/node/vitest-manager.ts` around lines 102 - 112, The loop uses find.any with last: getProjectRoot() redundantly when iterating potentialConfigFileLocations; replace the find.any + existsSync pattern by constructing the path with path.join(location, file) and checking existsSync(joinedPath), then use that path for readFileSync and dirname assignments (affecting potentialConfigFileLocations, configFiles, firstVitestConfig, vitestWorkspaceConfig and the usage of readFileSync/dirname); remove the find.any call and its getProjectRoot dependency to simplify and slightly improve performance.code/lib/create-storybook/src/commands/AddonConfigurationCommand.ts (1)
8-9: Direct imports from addon source directories.These relative imports cross package boundaries within the monorepo. This pattern works but creates tight coupling. Consider whether these should be exposed through the addon's public API instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/create-storybook/src/commands/AddonConfigurationCommand.ts` around lines 8 - 9, The code in AddonConfigurationCommand.ts imports addonA11yPostinstall and addonVitestPostinstall via relative paths into addon source trees, creating cross-package coupling; change these to import the postinstall functions from each addon's public API (re-export the postinstall symbols from the addon's package entry if not already exported) and update AddonConfigurationCommand.ts to import from the addon package name (e.g., the addon's main export) instead of '../../../../addons/.../postinstall' so consumers use the public surface rather than internal file paths.code/core/src/core-server/server-channel/telemetry-channel.test.ts (1)
3-3: Add explicit file extension to the relative import.As per coding guidelines, TypeScript files should use explicit file extensions for relative imports.
♻️ Suggested fix
-import { makePayload } from './telemetry-channel'; +import { makePayload } from './telemetry-channel.ts';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/core-server/server-channel/telemetry-channel.test.ts` at line 3, Update the relative import to include the explicit TypeScript file extension: replace the import of makePayload from './telemetry-channel' with an explicit extension (e.g., './telemetry-channel.ts') in telemetry-channel.test.ts so the import statement points to the exact module file; ensure the symbol makePayload remains imported from that updated path.code/core/src/telemetry/telemetry.ts (1)
14-14: Use explicit extensions on the new relative imports in this change set.Line 14 is one instance; the same repo rule also applies to the new imports in
code/core/src/telemetry/types.ts,code/core/src/core-server/presets/common-preset.ts, andcode/core/src/telemetry/detect-agent.test.ts.As per coding guidelines,
**/*.{ts,tsx}: Use explicit file extensions for relative code imports and exports in TypeScript (e.g.,./foo.tsor./bar.tsx).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/telemetry/telemetry.ts` at line 14, Update the relative TypeScript imports to include explicit file extensions: change the import of detectAgent (import { detectAgent } from './detect-agent') in telemetry.ts to use './detect-agent.ts' and apply the same fix for the new relative imports added in code/core/src/telemetry/types.ts, code/core/src/core-server/presets/common-preset.ts, and code/core/src/telemetry/detect-agent.test.ts so all relative imports/exports follow the repo rule of using explicit .ts/.tsx extensions.code/core/src/telemetry/detect-agent.ts (1)
1-8:'opencode'is declared but never produced
KnownAgentNameincludes'opencode', but no detection branch returns it. This creates type/runtime drift and makes downstream handling misleading. Either add detection for it (e.g., specific env marker orAGENT === 'opencode') or remove it from the union until supported.Proposed fix (if `AGENT=opencode` is intended)
function detectExplicitAgent(env: NodeJS.ProcessEnv): AgentInfo | undefined { @@ + // OpenCode + if (env.AGENT === 'opencode') { + return { + name: 'opencode', + }; + } + // Amp if (env.AGENT === 'amp') {Also applies to: 21-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/telemetry/detect-agent.ts` around lines 1 - 8, KnownAgentName includes 'opencode' but no code returns it; either remove 'opencode' from the KnownAgentName union or add a detection branch that actually yields 'opencode' (e.g., check for process.env.AGENT === 'opencode' or a specific env/marker) in the agent detection logic (the function that maps runtime markers to KnownAgentName). Update the type or detection function accordingly so the union and runtime detection stay in sync (refer to KnownAgentName and the agent-detection function that returns KnownAgentName).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a6a0b8b9-93f0-4ee9-8ff4-3cc437196664
⛔ Files ignored due to path filters (2)
.yarn/patches/@testing-library-user-event-npm-14.6.1-5da7e1d4e2.patchis excluded by!**/.yarn/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (141)
CHANGELOG.mdcode/.eslintrc.jscode/addons/a11y/package.jsoncode/addons/a11y/src/postinstall.tscode/addons/docs/package.jsoncode/addons/links/package.jsoncode/addons/onboarding/package.jsoncode/addons/pseudo-states/package.jsoncode/addons/themes/package.jsoncode/addons/vitest/package.jsoncode/addons/vitest/src/node/vitest-manager.tscode/addons/vitest/src/postinstall.test.tscode/addons/vitest/src/postinstall.tscode/addons/vitest/src/typings.d.tscode/addons/vitest/src/updateVitestFile.test.tscode/addons/vitest/src/updateVitestFile.tscode/addons/vitest/src/vitest-plugin/index.tscode/builders/builder-vite/package.jsoncode/builders/builder-vite/src/build.tscode/builders/builder-vite/src/plugins/vite-inject-mocker/plugin.test.tscode/builders/builder-vite/src/plugins/vite-inject-mocker/plugin.tscode/builders/builder-vite/src/plugins/vite-mock/plugin.tscode/builders/builder-webpack5/package.jsoncode/builders/builder-webpack5/src/plugins/webpack-mock-plugin.tscode/core/build-config.tscode/core/package.jsoncode/core/src/bin/core.tscode/core/src/cli/AddonVitestService.test.tscode/core/src/cli/AddonVitestService.tscode/core/src/common/js-package-manager/JsPackageManager.tscode/core/src/common/js-package-manager/PNPMProxy.tscode/core/src/common/utils/get-storybook-refs.test.tscode/core/src/common/utils/get-storybook-refs.tscode/core/src/common/utils/resolve-path-in-sb-cache.test.tscode/core/src/common/utils/resolve-path-in-sb-cache.tscode/core/src/common/versions.tscode/core/src/components/components/Select/Select.stories.tsxcode/core/src/components/components/Select/Select.tsxcode/core/src/core-events/index.tscode/core/src/core-server/presets/common-preset.tscode/core/src/core-server/server-channel/telemetry-channel.test.tscode/core/src/core-server/server-channel/telemetry-channel.tscode/core/src/core-server/utils/checklist.tscode/core/src/csf-tools/CsfFile.test.tscode/core/src/csf-tools/CsfFile.tscode/core/src/manager-api/modules/refs.tscode/core/src/manager-api/modules/url.tscode/core/src/manager-api/tests/refs.test.tscode/core/src/manager-api/tests/url.test.jscode/core/src/manager-api/version.tscode/core/src/manager/components/preview/Viewport.tsxcode/core/src/manager/components/preview/tools/share.stories.tsxcode/core/src/manager/components/preview/tools/share.tsxcode/core/src/manager/components/preview/tools/zoom.stories.tsxcode/core/src/manager/components/preview/tools/zoom.tsxcode/core/src/manager/globals/exports.tscode/core/src/mocking-utils/index.tscode/core/src/mocking-utils/mocker-runtime.jscode/core/src/mocking-utils/redirect.tscode/core/src/mocking-utils/runtime.tscode/core/src/node-logger/index.test.tscode/core/src/node-logger/index.tscode/core/src/server-errors.tscode/core/src/telemetry/detect-agent.test.tscode/core/src/telemetry/detect-agent.tscode/core/src/telemetry/storybook-metadata.tscode/core/src/telemetry/telemetry.tscode/core/src/telemetry/types.tscode/core/src/toolbar/components/ToolbarMenuSelect.tsxcode/e2e-tests/preview-api.spec.tscode/frameworks/angular/build-schema.jsoncode/frameworks/angular/package.jsoncode/frameworks/angular/start-schema.jsoncode/frameworks/ember/package.jsoncode/frameworks/html-vite/package.jsoncode/frameworks/nextjs-vite/package.jsoncode/frameworks/nextjs/build-config.tscode/frameworks/nextjs/package.jsoncode/frameworks/nextjs/src/aliases/webpack.tscode/frameworks/nextjs/src/export-mocks/link/index.tsxcode/frameworks/nextjs/src/export-mocks/webpack.tscode/frameworks/preact-vite/package.jsoncode/frameworks/react-native-web-vite/package.jsoncode/frameworks/react-vite/package.jsoncode/frameworks/react-webpack5/package.jsoncode/frameworks/server-webpack5/package.jsoncode/frameworks/svelte-vite/package.jsoncode/frameworks/sveltekit/package.jsoncode/frameworks/vue3-vite/package.jsoncode/frameworks/web-components-vite/package.jsoncode/lib/cli-sb/package.jsoncode/lib/cli-storybook/package.jsoncode/lib/cli-storybook/src/automigrate/index.tscode/lib/cli-storybook/src/automigrate/multi-project.tscode/lib/cli-storybook/src/automigrate/types.tscode/lib/cli-storybook/src/bin/run.tscode/lib/cli-storybook/src/codemod/csf-factories.tscode/lib/cli-storybook/src/codemod/helpers/config-to-csf-factory.test.tscode/lib/cli-storybook/src/codemod/helpers/config-to-csf-factory.tscode/lib/cli-storybook/src/codemod/helpers/csf-factories-utils.tscode/lib/cli-storybook/src/codemod/helpers/story-to-csf-factory.test.tscode/lib/cli-storybook/src/codemod/helpers/story-to-csf-factory.tscode/lib/cli-storybook/src/sandbox.tscode/lib/codemod/package.jsoncode/lib/codemod/src/index.test.tscode/lib/codemod/src/index.tscode/lib/core-webpack/package.jsoncode/lib/create-storybook/package.jsoncode/lib/create-storybook/src/commands/AddonConfigurationCommand.test.tscode/lib/create-storybook/src/commands/AddonConfigurationCommand.tscode/lib/create-storybook/src/commands/ProjectDetectionCommand.tscode/lib/create-storybook/src/initiate.tscode/lib/create-storybook/src/scaffold-new-project.tscode/lib/create-storybook/src/services/VersionService.test.tscode/lib/create-storybook/src/services/VersionService.tscode/lib/csf-plugin/package.jsoncode/lib/eslint-plugin/package.jsoncode/lib/react-dom-shim/package.jsoncode/package.jsoncode/presets/create-react-app/package.jsoncode/presets/react-webpack/package.jsoncode/presets/server-webpack/package.jsoncode/renderers/html/package.jsoncode/renderers/preact/package.jsoncode/renderers/react/package.jsoncode/renderers/server/package.jsoncode/renderers/svelte/package.jsoncode/renderers/vue3/package.jsoncode/renderers/web-components/package.jsondocs/_snippets/csf-factories-automigrate-with-config-directory.mddocs/_snippets/storybook-preview-configure-globaltypes.mddocs/api/cli-options.mdxdocs/api/csf/csf-next.mdxdocs/get-started/frameworks/angular.mdxdocs/versions/latest.jsondocs/versions/next.jsondocs/writing-stories/typescript.mdxscripts/bench/bench-packages.tsscripts/build/utils/entry-utils.tsscripts/build/utils/generate-bundle.tsscripts/package.json
💤 Files with no reviewable changes (2)
- code/core/src/server-errors.ts
- code/lib/create-storybook/src/initiate.ts
| // Our codebase is mostly TypeScript, and typescript will warn when imports are not found. | ||
| // It IS set to 'error' for JS files. | ||
| 'import-x/named': 'warn', | ||
| 'import-x/named': 'off', |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In eslint-plugin-import-x (or eslint-plugin-import equivalent), does the named rule catch invalid named imports that TypeScript may miss (especially when importing from untyped JS/CJS modules)? Please provide current docs/recommendations.
💡 Result:
Yes, the import-x/named rule in eslint-plugin-import-x (and the equivalent import/named in eslint-plugin-import) catches invalid named imports from untyped JS/CJS modules that TypeScript may miss. The rule statically analyzes the targeted module file to verify that named imports match actual named exports declared there, regardless of TypeScript typing. Key points from official docs: - It verifies "named imports are part of the set of named exports in the referenced module" by inspecting the file contents {1, high}. - For untyped JS/CJS modules (especially node_modules), it checks package.json for jsnext:main (deprecated) or module fields to find the ES module entry, then analyzes exports {1, high}. - Modules can be ignored via import-x/ignore setting (defaults include node_modules), but when not ignored, it analyzes them {1, high}. - Note: "A module path that is ignored or not unambiguously an ES module will not be reported" {1, high}. CJS modules with module.exports may not be treated as ES modules, potentially skipping analysis, but pure JS ES modules or those with module field are checked. - In TypeScript configs (flat/typescript, typescript), the rule is disabled (🚫) because TypeScript handles named import validation via types, but it can be manually enabled and works with eslint-import-resolver-typescript for path resolution {1,3,4,5, high}. TypeScript itself may miss runtime exports in untyped JS if no .d.ts declares them exhaustively. Current recommendations (2026): - Use eslint-plugin-import-x with flat/recommended or recommended configs where named is enabled by default {3,4,5, high}. - For TS projects, extend plugin:import-x/typescript + resolver, but enable import-x/named explicitly if you want checks on untyped JS {4,16, high}. - Pair with eslint-import-resolver-typescript to properly resolve paths and prefer @types but fall back to JS analysis {16, high}.
Citations:
- 1: https://github.com/un-ts/eslint-plugin-import-x/blob/master/docs/rules/named.md
- 2: https://registry.npmjs.org/eslint-plugin-import-x
- 3: https://github.com/un-ts/eslint-plugin-import-x
- 4: https://www.npmjs.com/package/eslint-plugin-import-x
- 5: https://github.com/import-js/eslint-import-resolver-typescript
🏁 Script executed:
cat -n code/.eslintrc.js | sed -n '150,180p'Repository: storybookjs/storybook
Length of output: 1433
🏁 Script executed:
rg -A 5 -B 5 "import-x/named" code/.eslintrc.jsRepository: storybookjs/storybook
Length of output: 413
🏁 Script executed:
rg -n "^\s*rules:\s*{" code/.eslintrc.js | head -20Repository: storybookjs/storybook
Length of output: 156
🏁 Script executed:
rg -n "rules:" code/.eslintrc.js | head -20Repository: storybookjs/storybook
Length of output: 379
🏁 Script executed:
sed -n '16,68p' code/.eslintrc.jsRepository: storybookjs/storybook
Length of output: 2247
Clarify the misleading comment about JS file configuration
Lines 163–164: The comment states "It IS set to 'error' for JS files," but this ESLint config has no explicit import-x/named rule for JS files. The rule is only mentioned here, where it's disabled for TypeScript files. Revise the comment to accurately reflect what the config does—disable the check for TypeScript (where TypeScript validates named imports) while relying on plugin defaults for JS files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/.eslintrc.js` around lines 163 - 165, Update the misleading comment
above the 'import-x/named' rule to state that the rule is disabled for
TypeScript files because the TypeScript compiler handles named-import
validation, and that JS behavior will follow plugin/default settings (i.e.,
there is no explicit 'error' setting for JS here); reference the
'import-x/named' entry so reviewers know which rule the comment describes and
ensure the revised comment clearly distinguishes TypeScript vs JS handling.
| @@ -0,0 +1,49 @@ | |||
| import { describe, expect, it } from 'vitest'; | |||
|
|
|||
| import { isConfigAlreadySetup } from './postinstall'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Missing file extension on relative import.
As per coding guidelines, TypeScript files should use explicit file extensions for relative imports.
🔧 Suggested fix
-import { isConfigAlreadySetup } from './postinstall';
+import { isConfigAlreadySetup } from './postinstall.ts';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { isConfigAlreadySetup } from './postinstall'; | |
| import { isConfigAlreadySetup } from './postinstall.ts'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/addons/vitest/src/postinstall.test.ts` at line 3, The import in
postinstall.test.ts uses a relative path without an explicit extension; update
the import statement that references isConfigAlreadySetup so it includes the
TypeScript extension (change './postinstall' to './postinstall.ts') to comply
with the TypeScript explicit-extension guideline and ensure module resolution
for the isConfigAlreadySetup import works correctly.
| config: (config) => ({ | ||
| ...config, | ||
| build: { | ||
| outDir: options.outputDir, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
The config hook incorrectly spreads config while overwriting build.
The spread ...config followed by a new build object loses all other build.* properties (e.g., emptyOutDir, rollupOptions). Vite's config hooks perform deep merging on the returned object, so you only need to return the partial override.
Proposed fix
config: (config) => ({
- ...config,
build: {
outDir: options.outputDir,
},
}),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/builders/builder-vite/src/build.ts` around lines 50 - 55, The config
hook is incorrectly spreading the incoming config which then fully replaces
nested build properties; in the config: (config) => ({ ...config, build: {
outDir: options.outputDir } }) block, remove the spread and return only the
partial override so Vite's deep merge preserves existing build.* fields—i.e.,
change the config hook to return { build: { outDir: options.outputDir } }
(referencing the config hook and options.outputDir).
| // configEnvironment is a new method in Vite 6 | ||
| // It is used to configure configs based on the environment | ||
| // E.g. Nitro uses this method to set the output directory to .output/public/ | ||
| configEnvironment: () => ({ | ||
| build: { | ||
| outDir: options.outputDir, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the Vite 6 configEnvironment plugin hook API signature?
💡 Result:
The Vite 6 configEnvironment plugin hook API signature is: configEnvironment(name: string, options: EnvironmentOptions): EnvironmentOptions | void | Promise<EnvironmentOptions | void> This hook is called for each environment after the config hook, receiving the environment name and its partially resolved options. Plugins can mutate options in place or return partial EnvironmentOptions to merge. Kind: async, sequential Example from official docs: configEnvironment(name: string, options: EnvironmentOptions) { if (name === 'rsc') { return { resolve: { conditions: ['workerd'] } } } } EnvironmentOptions interface includes: interface EnvironmentOptions { define?: Record<string, any> resolve?: EnvironmentResolveOptions optimizeDeps?: DepOptimizationOptions consumer?: 'client' | 'server' dev?: DevOptions build?: BuildOptions } Sources confirm this across Vite 6 docs and main branch (Vite 6+).
Citations:
- 1: https://v6.vite.dev/guide/api-environment-plugins
- 2: https://vite.dev/guide/api-environment-plugins
- 3: https://main.vite.dev/guide/api-environment-plugins
🏁 Script executed:
# Find and read the package.json for builder-vite
find . -path "*/code/builders/builder-vite/package.json" -type f -exec cat {} \;Repository: storybookjs/storybook
Length of output: 1764
🏁 Script executed:
# Search for all usages of configEnvironment in the codebase
rg "configEnvironment" --type ts --type tsx -B 2 -A 5Repository: storybookjs/storybook
Length of output: 92
🏁 Script executed:
# Search for configEnvironment usages across TypeScript files
rg "configEnvironment" -B 2 -A 5Repository: storybookjs/storybook
Length of output: 823
🏁 Script executed:
# Check the build.ts file around the lines in question to understand context
sed -n '50,70p' code/builders/builder-vite/src/build.tsRepository: storybookjs/storybook
Length of output: 636
Fix configEnvironment hook signature to accept required parameters.
The configEnvironment hook signature is confirmed as a Vite 6 feature with API: configEnvironment(name: string, options: EnvironmentOptions). The current implementation configEnvironment: () => ({...}) does not accept the required name and options parameters. Update to: configEnvironment: (name, options) => ({ build: { outDir: options.outputDir } }) to properly implement the Vite 6 plugin hook contract. Storybook's Vite peer dependency correctly includes Vite 6 (^6.0.0).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/builders/builder-vite/src/build.ts` around lines 56 - 63, The
configEnvironment hook currently uses the zero-arg signature but Vite 6 requires
configEnvironment(name: string, options: EnvironmentOptions); update the
exported plugin's configEnvironment implementation (symbol: configEnvironment)
to accept (name, options) and return the build override using options.outputDir
(e.g., configEnvironment: (name, options) => ({ build: { outDir:
options.outputDir } })), ensuring the hook matches the Vite 6 plugin contract
and uses options.outputDir rather than relying on closure-only values.
| @@ -0,0 +1,35 @@ | |||
| import { afterEach, describe, expect, it, vi } from 'vitest'; | |||
|
|
|||
| import { checkRef } from './get-storybook-refs'; | |||
There was a problem hiding this comment.
Use explicit extension for this relative TypeScript import.
Please include the .ts extension in this import path.
🔧 Proposed fix
-import { checkRef } from './get-storybook-refs';
+import { checkRef } from './get-storybook-refs.ts';As per coding guidelines, **/*.{ts,tsx} must use explicit file extensions for relative imports/exports.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { checkRef } from './get-storybook-refs'; | |
| import { checkRef } from './get-storybook-refs.ts'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/core/src/common/utils/get-storybook-refs.test.ts` at line 3, The import
in get-storybook-refs.test.ts uses a relative TypeScript path without an
explicit extension; update the import statement for checkRef to reference
'./get-storybook-refs.ts' (include the .ts extension) and ensure any other
relative imports/exports in this test file follow the same explicit .ts/.tsx
rule; verify the test still compiles after changing the import.
| const { dryRun, packageManager, yes, glob, ...codemodOptions } = options; | ||
| try { | ||
| let globString = '{stories,src}/**/{Button,Header,Page,button,header,page}.stories.*'; | ||
| if (!optionalEnvToBoolean(process.env.IN_STORYBOOK_SANDBOX)) { | ||
| const inSandbox = optionalEnvToBoolean(process.env.IN_STORYBOOK_SANDBOX) ?? false; | ||
| let globString = glob ?? '**/*.{stories,story}.{js,jsx,ts,tsx,mjs,mjsx,mts,mtsx}'; | ||
|
|
||
| if (!glob && inSandbox) { | ||
| // Sandbox uses limited glob for faster testing (unless glob explicitly provided) | ||
| globString = '{stories,src}/**/{Button,Header,Page,button,header,page}.stories.*'; | ||
| } else if (!glob && !yes) { |
There was a problem hiding this comment.
This retry path never terminates for non-interactive runs.
If glob came from --glob, --yes suppresses prompts, or the sandbox default glob misses, the catch block just calls runStoriesCodemod(options) with the same inputs. That throws No files matched again and recurses forever. Please fail fast for non-interactive cases, or clear glob before retrying so the next pass can actually recover.
🛠️ Suggested fix
async function runStoriesCodemod(options: {
dryRun: boolean | undefined;
packageManager: JsPackageManager;
useSubPathImports: boolean;
previewConfigPath: string;
yes: boolean | undefined;
glob: string | undefined;
}) {
const { dryRun, packageManager, yes, glob, ...codemodOptions } = options;
+ const inSandbox = optionalEnvToBoolean(process.env.IN_STORYBOOK_SANDBOX) ?? false;
try {
- const inSandbox = optionalEnvToBoolean(process.env.IN_STORYBOOK_SANDBOX) ?? false;
let globString = glob ?? '**/*.{stories,story}.{js,jsx,ts,tsx,mjs,mjsx,mts,mtsx}';
if (!glob && inSandbox) {
// Sandbox uses limited glob for faster testing (unless glob explicitly provided)
globString = '{stories,src}/**/{Button,Header,Page,button,header,page}.stories.*';
@@
} catch (err: any) {
if (err.message === 'No files matched') {
- await runStoriesCodemod(options);
+ if (glob || yes || inSandbox) {
+ throw err;
+ }
+ await runStoriesCodemod({ ...options, glob: undefined });
} else {
throw err;
}
}
}Also applies to: 50-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/lib/cli-storybook/src/codemod/csf-factories.ts` around lines 25 - 33,
The retry in the catch of runStoriesCodemod causes an infinite loop when runs
are non-interactive or a glob was explicitly provided; update the catch so that
if options.yes is true or options.glob is present it fails immediately (rethrow
or return an error) instead of retrying, and only attempt a retry by clearing
options.glob (e.g., delete options.glob or set it to undefined) when the run is
interactive and no explicit glob was supplied; adjust the logic around
runStoriesCodemod(options), options.glob, and the IN_STORYBOOK_SANDBOX behavior
accordingly so a non-interactive invocation fails fast while interactive runs
can retry with a cleared glob.
| try { | ||
| logger.logBox( | ||
| [messages.welcome] | ||
| .concat(isOutdated && !isPrerelease ? [messages.notLatest] : []) | ||
| .concat(init && (isOutdated || isPrerelease) ? [messages.longInitTime] : []) | ||
| .concat(isPrerelease ? [messages.prerelease] : []) | ||
| .join('\n'), | ||
| { | ||
| rounded: true, | ||
| } | ||
| ); | ||
| } catch {} |
There was a problem hiding this comment.
Empty catch block silently swallows errors.
The try { ... } catch {} around logger.logBox completely silences any errors. While the intent seems to be preventing log rendering failures from blocking sandbox creation, silently discarding errors makes debugging difficult.
Consider logging at debug level to preserve visibility:
🛡️ Proposed fix to log caught errors
try {
logger.logBox(
[messages.welcome]
.concat(isOutdated && !isPrerelease ? [messages.notLatest] : [])
.concat(init && (isOutdated || isPrerelease) ? [messages.longInitTime] : [])
.concat(isPrerelease ? [messages.prerelease] : [])
.join('\n'),
{
rounded: true,
}
);
- } catch {}
+ } catch (err) {
+ logger.debug(`Failed to render welcome box: ${err}`);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| logger.logBox( | |
| [messages.welcome] | |
| .concat(isOutdated && !isPrerelease ? [messages.notLatest] : []) | |
| .concat(init && (isOutdated || isPrerelease) ? [messages.longInitTime] : []) | |
| .concat(isPrerelease ? [messages.prerelease] : []) | |
| .join('\n'), | |
| { | |
| rounded: true, | |
| } | |
| ); | |
| } catch {} | |
| try { | |
| logger.logBox( | |
| [messages.welcome] | |
| .concat(isOutdated && !isPrerelease ? [messages.notLatest] : []) | |
| .concat(init && (isOutdated || isPrerelease) ? [messages.longInitTime] : []) | |
| .concat(isPrerelease ? [messages.prerelease] : []) | |
| .join('\n'), | |
| { | |
| rounded: true, | |
| } | |
| ); | |
| } catch (err) { | |
| logger.debug(`Failed to render welcome box: ${err}`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/lib/cli-storybook/src/sandbox.ts` around lines 83 - 94, The empty catch
around logger.logBox swallows errors; update the catch to log the caught error
(e.g., using logger.debug or logger.error) so failures in the welcome box are
visible but do not throw—keep the try/catch to avoid blocking sandbox creation,
but in the catch callback call logger.debug or logger.error with a clear message
and include the caught error object; locate the instance of logger.logBox in
sandbox.ts (the block that builds
messages.welcome/messages.notLatest/messages.longInitTime/messages.prerelease)
and replace the empty catch with a call that records the error for debugging
while preserving existing behavior.
| const rsbuildMatch = ancestor.command?.match(/(?:^|\s)create[\s\-]rsbuild/i); | ||
| if (rsbuildMatch) { | ||
| return 'create-rsbuild'; | ||
| } | ||
| // Check for @tanstack/start | ||
| if (ancestor.command?.includes('@tanstack/start')) { | ||
| return '@tanstack/start'; |
There was a problem hiding this comment.
Tighten matcher boundaries to prevent false CLI attribution.
Line 59 and Line 64 can overmatch (create-rsbuilder, @tanstack/startup) and return incorrect integration values.
Suggested fix
- const rsbuildMatch = ancestor.command?.match(/(?:^|\s)create[\s\-]rsbuild/i);
+ const rsbuildMatch = ancestor.command?.match(
+ /(?:^|\s)create[\s\-]rsbuild(?:@[^ ]+)?(?:\s|$)/i
+ );
if (rsbuildMatch) {
return 'create-rsbuild';
}
// Check for `@tanstack/start`
- if (ancestor.command?.includes('@tanstack/start')) {
+ if (/(?:^|\s)@tanstack\/start(?:@[^ ]+)?(?:\s|$)/i.test(ancestor.command ?? '')) {
return '@tanstack/start';
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/lib/create-storybook/src/services/VersionService.ts` around lines 59 -
65, The matcher logic in VersionService that checks ancestor.command is too
loose: change the rsbuild detection (currently using
ancestor.command?.match(/(?:^|\s)create[\s\-]rsbuild/i) referenced as
rsbuildMatch) to require exact token boundaries (e.g. use word-boundary or
anchor-based regex like requiring end or whitespace after "rsbuild") so it won't
match things like "create-rsbuilder"; likewise replace the simple
includes('@tanstack/start') check with a boundary-aware test (regex or token
split) that ensures "@tanstack/start" is a standalone token (not part of "startu
p" or "startup"); update both checks in VersionService to use these stricter
regex/token checks.
| ## Vue specific tips | ||
|
|
||
| Vue has excellent support for TypeScript, and we have done our utmost to take advantage of that in the stories files. For example, consider the following strongly typed Vue 3 single file component (SFC): | ||
| ## Vue specific tips |
There was a problem hiding this comment.
Minor grammar fixes: use hyphens in compound modifiers.
The static analysis correctly identifies two hyphenation issues:
- Line 73: "Vue specific tips" → "Vue-specific tips" (compound adjective)
- Line 96: "type check SFC files" → "type-check SFC files" (compound verb)
📝 Proposed fix
-## Vue specific tips
+## Vue-specific tips-You can type check SFC files with `vue-tsc` and get editor support in VSCode by installing the official [Vue extension](https://marketplace.visualstudio.com/items?itemName=Vue.volar).
+You can type-check SFC files with `vue-tsc` and get editor support in VSCode by installing the official [Vue extension](https://marketplace.visualstudio.com/items?itemName=Vue.volar).Also applies to: 96-96
🧰 Tools
🪛 LanguageTool
[grammar] ~73-~73: Use a hyphen to join words.
Context: ...*/} ## Vue specific tips Vue has excellent support...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/writing-stories/typescript.mdx` at line 73, Update the two compound
modifiers to use hyphens: change the heading text "Vue specific tips" to
"Vue-specific tips" and update the sentence "type check SFC files" to
"type-check SFC files"; locate the occurrences by searching for those exact
phrases in the document (e.g., the heading string and the sentence containing
"type check SFC files") and replace them accordingly.
| '-b, --base-branch [branchName]', | ||
| 'The base branch to compare the results with. Requires GCP_CREDENTIALS env var' | ||
| ) | ||
| .option( | ||
| '-p, --pull-request <number>', | ||
| '-p, --pull-request [pullRequestNumber]', | ||
| 'The PR number to add compare results to. Only used together with --baseBranch', |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Commander.js, for .option('-b, --base-branch [branchName]'), what runtime value does program.opts().baseBranch have when the flag is present without an argument?
💡 Result:
In Commander.js, for .option('-b, --base-branch [branchName]'), when the flag is present without an argument (e.g., --base-branch), the runtime value of program.opts.baseBranch is true.
Citations:
- 1: Value inside angle brackets tj/commander.js#817
- 2: https://www.econewsq8.com/node_modules/commander/Readme.md
- 3: https://www.npmjs.com/package/commander/v/8.0.0-0
- 4: https://www.npmjs.com/package/commander
- 5: https://omniedge.io/node_modules/commander
- 6: https://www.npmjs.com/package/commander/v/4.1.0
- 7: https://github.com/tj/commander.js/blob/master/Readme.md
🏁 Script executed:
cd scripts/bench && head -n 520 bench-packages.ts | tail -n 120 | cat -nRepository: storybookjs/storybook
Length of output: 5097
--base-branch optional arg with [branchName] syntax resolves to boolean true at runtime, causing type mismatch.
When --base-branch is provided without a value (e.g., --base-branch alone), Commander.js sets options.baseBranch to true instead of a string. The code at line 446 properly checks typeof options.baseBranch === 'string', but line 498 uses a truthy check (if (options.baseBranch)) that passes for the boolean value, allowing compareResults() at line 499 to receive true instead of the expected string type. Additionally, if only --base-branch is used without a value and --upload is false, the GCP credentials validation at lines 446–51 is bypassed entirely.
Suggested fix
.option(
- '-b, --base-branch [branchName]',
+ '-b, --base-branch <branchName>',
'The base branch to compare the results with. Requires GCP_CREDENTIALS env var'
)Alternatively, replace the truthy check at line 498:
- if (options.baseBranch) {
+ if (typeof options.baseBranch === 'string') {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/bench/bench-packages.ts` around lines 407 - 412, The bug is that
options.baseBranch uses Commander optional-arg syntax so when passed without a
value it becomes boolean true; replace any truthy checks with an explicit type
check (e.g., use typeof options.baseBranch === 'string') before calling
compareResults() so compareResults() never receives boolean true, and ensure the
GCP_CREDENTIALS validation (the existing typeof options.baseBranch === 'string'
block) is used to gate both credential validation and the compareResults() call
(or normalize options.baseBranch to a string earlier) so the flow for
--base-branch with no value does not bypass the credentials check and does not
pass a boolean into compareResults().
|
The PR seems to be malformed. Please create a new one! |
8 participants
Closes #34431
What I did
The onboarding checklist telemetry calls (
onboarding-checklist-mutedandonboarding-checklist-status) were not respecting thedisableTelemetry: trueconfig option set in.storybook/main.ts.initializeChecklist()was called unconditionally incommon-preset.ts, and theonStateChangehandler inside it calledtelemetry()without any guard.Fix: Added a
disableTelemetryparameter toinitializeChecklist(). The caller incommon-preset.tsnow passescoreOptions?.disableTelemetry. When set, theonStateChangehandler returns early before any telemetry calls.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
disableTelemetry: trueto.storybook/main.tscoreconfigonboarding-checklist-mutedoronboarding-checklist-statustelemetry events are sentSummary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation