ReactNative: Migrate ondevice addons from addons#34500
Merged
ndelangen merged 10 commits intoApr 9, 2026
Merged
Conversation
5 tasks
- Add `deviceAddons` field to `StorybookConfigRaw` type in core-common.ts - Create `rn-ondevice-addons-to-device-addons` automigration fix that moves addons with 'ondevice' in their name from `addons` to `deviceAddons` in React Native Storybook projects - Register the new fix in the allFixes list - Add comprehensive tests (9 tests) for the check and run phases Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/4e6650e3-b5dc-4d8c-b331-2c0b7f9506e4 Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com>
Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/4e6650e3-b5dc-4d8c-b331-2c0b7f9506e4 Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add deviceAddons property to StorybookConfig type
Automigration: move RN on-device addons from Apr 8, 2026
addons to deviceAddons
ndelangen
reviewed
Apr 8, 2026
Per review feedback, deviceAddons is React Native-specific and should not live in the shared core type. The automigration fix uses ConfigFile string-path APIs so it works without the type definition. Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/98fca5c1-3428-4436-8892-f4254f66193f Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com>
ndelangen
reviewed
Apr 8, 2026
…eviceAddons Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/d4b80472-bc2f-4c03-a3e4-64604df8f1ec Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com>
Member
|
@copilot The Sb init command might be adding ondevice addons to the wrong field now, please check and correct if so. |
|
View your CI Pipeline Execution ↗ for commit f2e8e4a
☁️ Nx Cloud last updated this comment at |
ndelangen
changed the title
addons to deviceAddonsaddons
Contributor
Author
Checked the init flow. The The template contents are controlled by the |
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
Member
|
@copilot, I retargeted this PR to my feature branch. Can you resolve the merge conflicts that result, please? |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR appears to address React Native on-device addons migration documentation, but the diff also includes a large repo-wide shift to explicit .ts/.tsx relative import specifiers, plus new Builder-Vite module graph change detection wiring and some a11y/test-provider announcements.
Changes:
- Standardize internal TypeScript relative imports/exports to include explicit file extensions and enforce via ESLint.
- Builder-Vite: add module graph conversion +
onModuleGraphChangeimplementation with polling/debounce and tests. - Add live region announcements for test-provider state changes and add stories/tests around these behaviors.
Reviewed changes
Copilot reviewed 294 out of 1797 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| code/core/src/common/utils/load-main-config.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/utils/interpret-require.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/utils/get-storybook-refs.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/utils/get-storybook-refs.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/utils/get-storybook-info.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/utils/get-storybook-configuration.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/utils/get-story-id.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/utils/get-story-id.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/utils/get-renderer-name.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/utils/get-renderer-name.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/utils/get-framework-name.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/utils/get-framework-name.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/utils/get-addon-names.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/utils/get-addon-names.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/utils/get-addon-annotations.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/utils/get-addon-annotations.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/utils/formatter.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/utils/file-cache.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/utils/envs.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/utils/cli.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/utils/cli.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/utils/cache.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/utils/tests/template.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/utils/tests/paths.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/utils/tests/normalize-stories.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/utils/tests/interpret-files.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/presets.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/presets.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/js-package-manager/index.ts | Switch re-exports to explicit .ts extensions |
| code/core/src/common/js-package-manager/Yarn2Proxy.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/js-package-manager/Yarn2Proxy.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/js-package-manager/Yarn1Proxy.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/js-package-manager/Yarn1Proxy.test.ts | Update test imports/mocks to explicit .ts extensions |
| code/core/src/common/js-package-manager/PNPMProxy.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/js-package-manager/PNPMProxy.test.ts | Update test imports/mocks to explicit .ts extensions |
| code/core/src/common/js-package-manager/NPMProxy.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/js-package-manager/NPMProxy.test.ts | Update test imports/mocks to explicit .ts extensions |
| code/core/src/common/js-package-manager/JsPackageManagerFactory.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/js-package-manager/JsPackageManagerFactory.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/js-package-manager/JsPackageManager.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/js-package-manager/JsPackageManager.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/common/js-package-manager/BUNProxy.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/common/index.ts | Switch exports/imports to explicit .ts extensions |
| code/core/src/common/config.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/client-logger/index.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/cli/index.ts | Switch exports to explicit .ts extensions |
| code/core/src/cli/helpers.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/cli/helpers.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/cli/globalSettings.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/cli/globalSettings.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/cli/eslintPlugin.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/cli/eslintPlugin.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/cli/dirs.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/cli/NpmOptions.ts | Switch type import to explicit .ts extensions |
| code/core/src/cli/AddonVitestService.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/cli/AddonVitestService.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/cli/AddonVitestService.constants.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/channels/websocket/index.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/channels/postmessage/index.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/channels/main.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/channels/index.ts | Switch imports/exports to explicit .ts extensions |
| code/core/src/channels/index.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/builder-manager/utils/template.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/builder-manager/utils/files.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/builder-manager/utils/files.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/builder-manager/utils/data.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/builder-manager/index.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/backgrounds/types.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/backgrounds/preview.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/backgrounds/manager.tsx | Switch relative imports to explicit .ts/.tsx extensions |
| code/core/src/backgrounds/defaults.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/backgrounds/decorator.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/backgrounds/components/Tool.tsx | Switch relative imports to explicit .ts extensions |
| code/core/src/babel/vitest-config-helpers.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/babel/vitest-config-helpers.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/babel/index.ts | Switch exports to explicit .ts extensions |
| code/core/src/babel/expression-resolver.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/actions/runtime/index.ts | Switch exports to explicit .ts extensions |
| code/core/src/actions/runtime/configureActions.ts | Switch type import to explicit .ts extensions |
| code/core/src/actions/runtime/actions.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/actions/runtime/action.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/actions/preview.ts | Switch relative imports to explicit .ts extensions |
| code/core/src/actions/models/index.ts | Switch exports to explicit .ts extensions |
| code/core/src/actions/models/HandlerFunction.test-d.ts | Update type-test import to explicit .ts extensions |
| code/core/src/actions/models/ActionsMap.ts | Switch type import to explicit .ts extensions |
| code/core/src/actions/models/ActionsFunction.ts | Switch type imports to explicit .ts extensions |
| code/core/src/actions/models/ActionDisplay.ts | Switch type import to explicit .ts extensions |
| code/core/src/actions/manager.tsx | Switch relative imports to explicit .ts/.tsx extensions |
| code/core/src/actions/loaders.ts | Switch relative import to explicit .ts extensions |
| code/core/src/actions/index.ts | Switch exports to explicit .ts extensions |
| code/core/src/actions/decorator.ts | Switch relative import to explicit .ts extensions |
| code/core/src/actions/containers/ActionLogger/index.tsx | Fix action accumulation logic and switch imports to explicit extensions |
| code/core/src/actions/containers/ActionLogger/ActionLogger.stories.tsx | Add stories for ActionLogger container behavior |
| code/core/src/actions/components/Title.tsx | Switch relative import to explicit .ts extensions |
| code/core/src/actions/components/ActionLogger/index.tsx | Switch imports to explicit .ts/.tsx extensions |
| code/core/src/actions/addArgsHelpers.ts | Switch relative import to explicit .ts extensions |
| code/core/src/actions/addArgsHelpers.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/actions/addArgs.ts | Switch relative import to explicit .ts extensions |
| code/core/src/__tests/storybook-error.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/__tests/server-errors.test.ts | Update test imports to explicit .ts extensions |
| code/core/src/__tests/preview-errors.test.ts | Update test imports to explicit .ts extensions |
| code/core/scripts/generate-source-files.ts | Switch relative imports to explicit .ts extensions |
| code/core/package.json | Bump prerelease version and add live-announcer deps |
| code/core/build-config.ts | Switch type import to explicit .ts extensions |
| code/builders/builder-webpack5/vitest.config.ts | Switch shared config import to explicit .ts |
| code/builders/builder-webpack5/src/preview/virtual-module-mapping.ts | Switch type import to explicit .ts extensions |
| code/builders/builder-webpack5/src/preview/iframe-webpack.config.ts | Switch imports to explicit .ts extensions |
| code/builders/builder-webpack5/src/presets/preview-preset.ts | Switch import to explicit .ts extensions |
| code/builders/builder-webpack5/src/presets/custom-webpack-preset.ts | Switch imports to explicit .ts extensions |
| code/builders/builder-webpack5/src/index.ts | Switch exports to explicit .ts extensions |
| code/builders/builder-webpack5/package.json | Bump prerelease version |
| code/builders/builder-webpack5/build-config.ts | Switch type import to explicit .ts |
| code/builders/builder-vite/vitest.config.ts | Switch shared config import to explicit .ts |
| code/builders/builder-vite/src/vite-server.ts | Switch imports to explicit .ts extensions |
| code/builders/builder-vite/src/vite-config.ts | Switch imports to explicit .ts extensions |
| code/builders/builder-vite/src/vite-config.test.ts | Update test imports to explicit .ts extensions |
| code/builders/builder-vite/src/utils/without-vite-plugins.test.ts | Update test import to explicit .ts |
| code/builders/builder-vite/src/utils/process-preview-annotation.test.ts | Update test imports to explicit .ts |
| code/builders/builder-vite/src/utils/has-vite-plugins.test.ts | Update test import to explicit .ts |
| code/builders/builder-vite/src/utils/build-module-graph.ts | Add conversion from Vite module graph to shared ModuleGraph |
| code/builders/builder-vite/src/utils/build-module-graph.test.ts | Add unit tests for buildModuleGraph |
| code/builders/builder-vite/src/transform-iframe-html.ts | Switch import to explicit .ts |
| code/builders/builder-vite/src/preset.ts | Switch imports to explicit .ts extensions |
| code/builders/builder-vite/src/plugins/webpack-stats-plugin.ts | Switch import to explicit .ts |
| code/builders/builder-vite/src/plugins/vite-mock/plugin.ts | Switch imports and update Vite hook shape |
| code/builders/builder-vite/src/plugins/vite-inject-mocker/plugin.ts | Update resolveId hook shape with filter/handler |
| code/builders/builder-vite/src/plugins/strip-story-hmr-boundaries.ts | Update transform hook shape and factor include regex |
| code/builders/builder-vite/src/plugins/storybook-runtime-plugin.ts | Switch import to explicit .ts |
| code/builders/builder-vite/src/plugins/storybook-project-annotations-plugin.ts | Switch imports to explicit .ts |
| code/builders/builder-vite/src/plugins/storybook-optimize-deps-plugin.ts | Switch imports to explicit .ts |
| code/builders/builder-vite/src/plugins/storybook-optimize-deps-plugin.test.ts | Update test import to explicit .ts |
| code/builders/builder-vite/src/plugins/storybook-external-globals-plugin.ts | Precompute globals regex and add transform filter |
| code/builders/builder-vite/src/plugins/storybook-external-globals-plugin.test.ts | Update test import to explicit .ts |
| code/builders/builder-vite/src/plugins/storybook-entry-plugin.ts | Switch imports to explicit .ts |
| code/builders/builder-vite/src/plugins/inject-export-order-plugin.ts | Update transform hook shape with filter/handler |
| code/builders/builder-vite/src/plugins/index.ts | Switch exports to explicit .ts |
| code/builders/builder-vite/src/plugins/code-generator-plugin.ts | Switch imports to explicit .ts |
| code/builders/builder-vite/src/index.ts | Add module graph change detection, listener API, and startup polling |
| code/builders/builder-vite/src/index.test.ts | Add tests for onModuleGraphChange and builder lifecycle |
| code/builders/builder-vite/src/codegen-project-annotations.ts | Switch imports and change hash implementation |
| code/builders/builder-vite/src/codegen-modern-iframe-script.ts | Switch imports to explicit .ts |
| code/builders/builder-vite/src/codegen-modern-iframe-script.test.ts | Update test imports to explicit .ts |
| code/builders/builder-vite/src/codegen-importfn-script.ts | Switch import to explicit .ts |
| code/builders/builder-vite/src/codegen-importfn-script.test.ts | Update test import to explicit .ts |
| code/builders/builder-vite/src/build.ts | Switch imports to explicit .ts |
| code/builders/builder-vite/package.json | Bump prerelease version |
| code/builders/builder-vite/build-config.ts | Switch type import to explicit .ts |
| code/addons/vitest/vitest.config.ts | Switch shared config import to explicit .ts |
| code/addons/vitest/src/vitest-plugin/viewports.test.ts | Update test import to explicit .ts |
| code/addons/vitest/src/vitest-plugin/utils.ts | Simplify requiresProjectAnnotations signature |
| code/addons/vitest/src/vitest-plugin/test-utils.ts | Switch import to explicit .ts |
| code/addons/vitest/src/vitest-plugin/setup-file.ts | Switch import to explicit .ts |
| code/addons/vitest/src/vitest-plugin/setup-file.test.ts | Update test import to explicit .ts |
| code/addons/vitest/src/vitest-plugin/index.ts | Switch imports and adjust setup-file selection |
| code/addons/vitest/src/utils.ts | Switch type import to explicit .ts |
| code/addons/vitest/src/use-test-provider-state.ts | Narrow status mapping types and add cast |
| code/addons/vitest/src/updateVitestFile.ts | Switch template imports to .ts?raw |
| code/addons/vitest/src/updateVitestFile.test.ts | Update test import to explicit .ts |
| code/addons/vitest/src/updateVitestFile.config.workspace.test.ts | Update test imports to explicit .ts |
| code/addons/vitest/src/updateVitestFile.config.test.ts | Update test imports to explicit .ts |
| code/addons/vitest/src/updateVitestFile.config.4.test.ts | Update test imports to explicit .ts |
| code/addons/vitest/src/updateVitestFile.config.3.2.test.ts | Update test imports to explicit .ts |
| code/addons/vitest/src/preset.ts | Switch imports to explicit .ts |
| code/addons/vitest/src/postinstall.ts | Switch imports to explicit .ts |
| code/addons/vitest/src/postinstall.test.ts | Update test import to explicit .ts |
| code/addons/vitest/src/node/vitest.ts | Switch imports to explicit .ts |
| code/addons/vitest/src/node/vitest-manager.ts | Switch imports to explicit .ts |
| code/addons/vitest/src/node/test-manager.ts | Switch imports and use shared error helper |
| code/addons/vitest/src/node/test-manager.test.ts | Update test imports to explicit .ts |
| code/addons/vitest/src/node/reporter.ts | Switch imports to explicit .ts |
| code/addons/vitest/src/node/coverage-reporter.ts | Switch imports to explicit .ts |
| code/addons/vitest/src/node/boot-test-runner.ts | Switch imports and standardize fatal error shape |
| code/addons/vitest/src/node/boot-test-runner.test.ts | Update test imports to explicit .ts |
| code/addons/vitest/src/manager.tsx | Switch imports to explicit .ts/.tsx |
| code/addons/vitest/src/manager-store.ts | Switch imports to explicit .ts |
| code/addons/vitest/src/manager-store.mock.ts | Switch imports to explicit .ts |
| code/addons/vitest/src/logger.ts | Switch import to explicit .ts |
| code/addons/vitest/src/constants.ts | Switch imports/exports to explicit .ts |
| code/addons/vitest/src/components/TestStatusIcon.stories.tsx | Switch import to explicit .tsx |
| code/addons/vitest/src/components/TestProviderRender.tsx | Add aria-live announcements and switch imports to explicit extensions |
| code/addons/vitest/src/components/TestProviderRender.stories.tsx | Add stories asserting announcements via live region matcher |
| code/addons/vitest/src/components/SidebarContextMenu.tsx | Switch imports to explicit .ts/.tsx |
| code/addons/vitest/src/components/RelativeTime.stories.tsx | Switch import to explicit .tsx |
| code/addons/vitest/src/components/GlobalErrorModal.tsx | Switch imports to explicit .ts |
| code/addons/vitest/src/components/GlobalErrorModal.stories.tsx | Switch imports to explicit .ts/.tsx |
| code/addons/vitest/src/components/Description.tsx | Switch imports to explicit .ts/.tsx |
| code/addons/vitest/package.json | Bump prerelease version |
| code/addons/vitest/build-config.ts | Switch type import to explicit .ts |
| code/addons/themes/vitest.config.ts | Switch shared config import to explicit .ts |
| code/addons/themes/src/theme-switcher.tsx | Switch imports to explicit .ts |
| code/addons/themes/src/preview.ts | Switch import to explicit .ts |
| code/addons/themes/src/manager.tsx | Switch imports to explicit .ts/.tsx |
| code/addons/themes/src/index.ts | Switch imports/exports to explicit .ts |
| code/addons/themes/src/decorators/provider.decorator.tsx | Switch imports to explicit .ts |
| code/addons/themes/src/decorators/index.ts | Switch exports to explicit .ts/.tsx |
| code/addons/themes/src/decorators/helpers.ts | Switch imports to explicit .ts |
| code/addons/themes/src/decorators/data-attribute.decorator.tsx | Switch imports to explicit .ts |
| code/addons/themes/src/decorators/class-name.decorator.tsx | Switch imports to explicit .ts |
| code/addons/themes/src/constants.ts | Switch type import to explicit .ts |
| code/addons/themes/package.json | Bump prerelease version |
| code/addons/themes/build-config.ts | Switch type import to explicit .ts |
| code/addons/pseudo-states/vitest.config.ts | Switch shared config import to explicit .ts |
| code/addons/pseudo-states/src/types.ts | Switch type import to explicit .ts |
| code/addons/pseudo-states/src/types.test-d.ts | Switch imports to explicit .ts |
| code/addons/pseudo-states/src/stories/ShadowRootWithPart.stories.tsx | Refactor to shared grid component and switch imports |
| code/addons/pseudo-states/src/stories/ShadowRoot.stories.tsx | Refactor to shared grid component and switch imports |
| code/addons/pseudo-states/src/stories/PseudoStateGrid.tsx | Add shared pseudo state grid component |
| code/addons/pseudo-states/src/stories/Portal.stories.tsx | Switch import to explicit .tsx |
| code/addons/pseudo-states/src/stories/NestedRules.stories.tsx | Switch import to explicit .tsx |
| code/addons/pseudo-states/src/stories/Input.stories.tsx | Switch import to explicit .tsx |
| code/addons/pseudo-states/src/stories/CustomElementNested.tsx | Switch import to explicit .tsx |
| code/addons/pseudo-states/src/stories/CustomElementNested.stories.tsx | Switch imports and refactor to shared grid |
| code/addons/pseudo-states/src/stories/CustomElement.stories.tsx | Switch imports and refactor to shared grid |
| code/addons/pseudo-states/src/stories/CSSAtRules.stories.tsx | Switch imports and refactor to shared grid |
| code/addons/pseudo-states/src/stories/Button.stories.tsx | Switch imports and refactor to shared grid |
| code/addons/pseudo-states/src/preview/withPseudoState.ts | Switch imports to explicit .ts |
| code/addons/pseudo-states/src/preview/splitSelectors.test.ts | Update test import to explicit .ts |
| code/addons/pseudo-states/src/preview/rewriteStyleSheet.ts | Switch imports to explicit .ts |
| code/addons/pseudo-states/src/preview/rewriteStyleSheet.test.ts | Update test imports to explicit .ts |
| code/addons/pseudo-states/src/preview.ts | Switch imports to explicit .ts |
| code/addons/pseudo-states/src/manager/PseudoStateTool.tsx | Switch import to explicit .ts |
| code/addons/pseudo-states/src/manager.ts | Switch imports to explicit .ts/.tsx |
| code/addons/pseudo-states/src/index.ts | Switch imports/exports to explicit .ts |
| code/addons/pseudo-states/package.json | Bump prerelease version |
| code/addons/pseudo-states/build-config.ts | Switch type import to explicit .ts |
| code/addons/onboarding/vitest.config.ts | Switch shared config import to explicit .ts |
| code/addons/onboarding/src/preset.ts | Switch import to explicit .ts |
| code/addons/onboarding/src/manager.tsx | Switch imports and lazy imports to explicit .tsx |
| code/addons/onboarding/src/features/SplashScreen/SplashScreen.stories.tsx | Switch import to explicit .tsx |
| code/addons/onboarding/src/features/IntentSurvey/IntentSurvey.tsx | Switch import to explicit .ts |
| code/addons/onboarding/src/features/IntentSurvey/IntentSurvey.stories.tsx | Switch import to explicit .tsx |
| code/addons/onboarding/src/components/List/ListItem/ListItem.tsx | Switch styled import to explicit .tsx |
| code/addons/onboarding/src/components/List/List.tsx | Switch styled import to explicit .tsx |
| code/addons/onboarding/src/components/List/List.stories.tsx | Switch imports to explicit .tsx |
| code/addons/onboarding/src/components/Confetti/Confetti.stories.tsx | Switch import to explicit .tsx |
| code/addons/onboarding/src/Survey.tsx | Switch imports to explicit .ts/.tsx |
| code/addons/onboarding/src/Onboarding.tsx | Switch imports to explicit .ts/.tsx |
| code/addons/onboarding/package.json | Bump prerelease version |
| code/addons/onboarding/example-stories/Button.stories.tsx | Switch import to explicit .tsx |
| code/addons/onboarding/build-config.ts | Switch type import to explicit .ts |
| code/addons/links/vitest.config.ts | Switch shared config import to explicit .ts |
| code/addons/links/src/utils.ts | Switch import to explicit .ts |
| code/addons/links/src/utils.test.ts | Update test import to explicit .ts |
| code/addons/links/src/react/index.ts | Switch import to explicit .tsx |
| code/addons/links/src/react/components/link.tsx | Switch import to explicit .ts |
| code/addons/links/src/react/components/link.test.tsx | Update test import to explicit .tsx |
| code/addons/links/src/preview.ts | Switch import to explicit .ts |
| code/addons/links/src/manager.ts | Switch import to explicit .ts |
| code/addons/links/src/index.ts | Switch imports/exports to explicit .ts |
| code/addons/links/package.json | Bump prerelease version |
| code/addons/links/build-config.ts | Switch type import to explicit .ts |
| code/addons/docs/src/mdx-plugin.ts | Update transform hook shape with filter/handler |
| code/addons/docs/package.json | Bump prerelease version |
| code/addons/docs/docs/props-tables.md | Fix duplicated word typo |
| code/addons/a11y/vitest.config.ts | Switch shared config import to explicit .ts |
| code/addons/a11y/src/withVisionSimulator.ts | Switch import to explicit .ts |
| code/addons/a11y/src/types.ts | Switch type import to explicit .ts |
| code/addons/a11y/src/preview.tsx | Switch imports to explicit .ts/.tsx |
| code/addons/a11y/src/preview.test.tsx | Update test imports/mocks to explicit .ts |
| code/addons/a11y/src/postinstall.ts | Switch type import to explicit .ts |
| code/addons/a11y/src/manager.tsx | Switch imports to explicit .ts/.tsx |
| code/addons/a11y/src/manager.test.tsx | Switch imports to explicit .ts/.tsx |
| code/addons/a11y/src/index.ts | Switch imports/exports to explicit .ts/.tsx |
| code/addons/a11y/src/components/VisionSimulator.tsx | Switch imports to explicit .ts |
| code/addons/a11y/src/components/VisionSimulator.stories.tsx | Switch imports to explicit .tsx |
| code/addons/a11y/src/components/TestDiscrepancyMessage.tsx | Switch import to explicit .ts |
| code/addons/a11y/src/components/TestDiscrepancyMessage.stories.tsx | Switch imports to explicit .tsx |
| code/addons/a11y/src/components/Tabs.tsx | Switch imports to explicit .ts/.tsx |
| code/addons/a11y/src/components/Report/Report.tsx | Switch imports to explicit .ts/.tsx |
| code/addons/a11y/src/components/Report/Report.stories.tsx | Switch imports to explicit .tsx |
| code/addons/a11y/src/components/Report/Details.tsx | Switch imports to explicit .ts/.tsx |
| code/addons/a11y/src/components/A11yContext.tsx | Avoid timer leaks on unmount and switch imports to explicit extensions |
| code/addons/a11y/src/components/A11yContext.test.tsx | Update test imports to explicit .ts/.tsx |
| code/addons/a11y/src/components/A11YPanel.tsx | Switch imports to explicit .ts/.tsx |
| code/addons/a11y/src/components/A11YPanel.test.tsx | Update test imports to explicit .ts/.tsx |
| code/addons/a11y/src/components/A11YPanel.stories.tsx | Switch imports to explicit .tsx |
| code/addons/a11y/src/axeRuleMappingHelper.ts | Switch imports to explicit .ts |
| code/addons/a11y/src/a11yRunnerUtils.ts | Switch imports to explicit .ts |
| code/addons/a11y/src/a11yRunnerUtils.test.ts | Update test import to explicit .ts |
| code/addons/a11y/src/a11yRunner.ts | Switch imports to explicit .ts |
| code/addons/a11y/src/a11yRunner.test.ts | Switch imports to explicit .ts |
| code/addons/a11y/package.json | Bump prerelease version |
| code/addons/a11y/build-config.ts | Switch type import to explicit .ts |
| code/.storybook/storybook.setup.ts | Add live region matcher import and switch preview import to .tsx |
| code/.storybook/preview.tsx | Register custom live region matcher and switch many imports to explicit extensions |
| code/.storybook/main.ts | Add stories directory for actions containers |
| code/.storybook/bench/bench.stories.tsx | Switch import to explicit .ts |
| code/.eslintrc.js | Enforce explicit extensions for TS/TSX relative imports |
| MIGRATION.md | Add RN on-device addon deviceAddons migration section |
| CHANGELOG.prerelease.md | Add prerelease notes for alpha.8 |
| CHANGELOG.md | Add 10.3.4/10.3.5 notes |
| AGENTS.md | Document preference for explicit file extensions in repo TS source |
| .vscode/settings.json | Disable MDX format-on-save; fix TypeScript SDK path |
| .oxfmtrc.json | Exclude docs/versions/*.json from formatter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+23
to
+28
| type TestStatusValue = Extract< | ||
| StatusValue, | ||
| `status-value:${'pending' | 'success' | 'error' | 'warning' | 'unknown'}` | ||
| >; | ||
|
|
||
| export type StatusValueToStoryIds = Record<TestStatusValue, StoryId[]>; |
There was a problem hiding this comment.
Casting status.value as TestStatusValue can cause a runtime crash if status.value is not one of the five allowed keys (e.g., if additional StatusValue variants exist for other status systems). In that case statusValueToStoryIdsMap[status.value] is undefined, and .push(...) will throw. Fix by guarding before pushing (e.g., check key existence), or by widening the map to include all possible StatusValue keys that can appear for these status types.
| return; | ||
| } | ||
| statusValueToStoryIdsMap[status.value].push(status.storyId); | ||
| statusValueToStoryIdsMap[status.value as TestStatusValue].push(status.storyId); |
There was a problem hiding this comment.
Casting status.value as TestStatusValue can cause a runtime crash if status.value is not one of the five allowed keys (e.g., if additional StatusValue variants exist for other status systems). In that case statusValueToStoryIdsMap[status.value] is undefined, and .push(...) will throw. Fix by guarding before pushing (e.g., check key existence), or by widening the map to include all possible StatusValue keys that can appear for these status types.
Comment on lines
+10
to
+21
| const getModuleFileFromMap = (viteModuleNode: { | ||
| file: string | null; | ||
| type: ViteModuleNode['type']; | ||
| importers: Set<ViteModuleNode>; | ||
| importedModules: Set<ViteModuleNode>; | ||
| }): string | undefined => { | ||
| for (const [filePath, viteModuleSet] of fileToModulesMap.entries()) { | ||
| if (viteModuleSet.has(viteModuleNode as ViteModuleNode)) { | ||
| return filePath; | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
getModuleFileFromMap() performs a full scan of fileToModulesMap and is called from inside the nested traversal of importers/importedModules. This can become O(N²) on large projects and will run on every change event. Consider precomputing a reverse lookup (e.g., WeakMap<ViteModuleNode, string>) once per build of the module graph, then do O(1) lookups during traversal.
Suggested change
| const getModuleFileFromMap = (viteModuleNode: { | |
| file: string | null; | |
| type: ViteModuleNode['type']; | |
| importers: Set<ViteModuleNode>; | |
| importedModules: Set<ViteModuleNode>; | |
| }): string | undefined => { | |
| for (const [filePath, viteModuleSet] of fileToModulesMap.entries()) { | |
| if (viteModuleSet.has(viteModuleNode as ViteModuleNode)) { | |
| return filePath; | |
| } | |
| } | |
| }; | |
| const moduleFileMap = new WeakMap<object, string>(); | |
| fileToModulesMap.forEach((viteModuleSet, filePath) => { | |
| viteModuleSet.forEach((viteModuleNode) => { | |
| moduleFileMap.set(viteModuleNode, filePath); | |
| }); | |
| }); | |
| const getModuleFileFromMap = (viteModuleNode: { | |
| file: string | null; | |
| type: ViteModuleNode['type']; | |
| importers: Set<ViteModuleNode>; | |
| importedModules: Set<ViteModuleNode>; | |
| }): string | undefined => moduleFileMap.get(viteModuleNode); |
Comment on lines
+108
to
+111
| const importPaths = new Set(Object.values(storyIndex.entries).map((entry) => entry.importPath)); | ||
|
|
||
| // Warm up the module graph for all story files | ||
| await Promise.all(Array.from(importPaths, (importPath) => server.warmupRequest(importPath))); |
There was a problem hiding this comment.
Promise.all() over all story import paths can create very high concurrency (and memory / CPU pressure) on large storybooks, potentially delaying startup or causing timeouts. Consider batching or limiting concurrency (e.g., chunk the paths or use a concurrency limiter) so change detection startup remains stable for large repos.
Comment on lines
+114
to
+156
| waitForModuleGraph = setInterval(() => { | ||
| void (async () => { | ||
| try { | ||
| if (!watcherChangeHandler) { | ||
| clearModuleGraphPolling(); | ||
| return; | ||
| } | ||
|
|
||
| if (process.hrtime(startTime)[0] > 30) { | ||
| clearModuleGraphPolling(); | ||
| const error = new Error( | ||
| 'Timed out while waiting for the Vite module graph to initialize' | ||
| ); | ||
| logger.error('Failed to complete Vite change detection startup'); | ||
| logger.error(error); | ||
| notifyListenersOfStartupFailure({ | ||
| type: 'unavailable', | ||
| reason: error.message, | ||
| error, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| if (server.moduleGraph.fileToModulesMap.size > 0) { | ||
| clearModuleGraphPolling(); | ||
| await server.waitForRequestsIdle(); | ||
| if (!watcherChangeHandler) { | ||
| return; | ||
| } | ||
|
|
||
| server.watcher.on('all', watcherChangeHandler); | ||
| watcherChangeHandler(); | ||
| } | ||
| } catch (error) { | ||
| clearModuleGraphPolling(); | ||
| logger.error('Failed to complete Vite change detection startup'); | ||
| logger.error(error instanceof Error ? error : String(error)); | ||
| notifyListenersOfStartupFailure({ | ||
| type: 'error', | ||
| error: error instanceof Error ? error : new Error(String(error)), | ||
| }); | ||
| } | ||
| })(); |
There was a problem hiding this comment.
This setInterval schedules an async IIFE every second; if the async body takes longer than the interval (e.g., due to waitForRequestsIdle()), overlapping executions are possible until clearModuleGraphPolling() happens. To avoid re-entrancy, consider switching to a setTimeout loop (schedule the next poll at the end of the current run) or add an inFlight guard.
Suggested change
| waitForModuleGraph = setInterval(() => { | |
| void (async () => { | |
| try { | |
| if (!watcherChangeHandler) { | |
| clearModuleGraphPolling(); | |
| return; | |
| } | |
| if (process.hrtime(startTime)[0] > 30) { | |
| clearModuleGraphPolling(); | |
| const error = new Error( | |
| 'Timed out while waiting for the Vite module graph to initialize' | |
| ); | |
| logger.error('Failed to complete Vite change detection startup'); | |
| logger.error(error); | |
| notifyListenersOfStartupFailure({ | |
| type: 'unavailable', | |
| reason: error.message, | |
| error, | |
| }); | |
| return; | |
| } | |
| if (server.moduleGraph.fileToModulesMap.size > 0) { | |
| clearModuleGraphPolling(); | |
| await server.waitForRequestsIdle(); | |
| if (!watcherChangeHandler) { | |
| return; | |
| } | |
| server.watcher.on('all', watcherChangeHandler); | |
| watcherChangeHandler(); | |
| } | |
| } catch (error) { | |
| clearModuleGraphPolling(); | |
| logger.error('Failed to complete Vite change detection startup'); | |
| logger.error(error instanceof Error ? error : String(error)); | |
| notifyListenersOfStartupFailure({ | |
| type: 'error', | |
| error: error instanceof Error ? error : new Error(String(error)), | |
| }); | |
| } | |
| })(); | |
| const pollForModuleGraph = async () => { | |
| try { | |
| if (!watcherChangeHandler) { | |
| clearModuleGraphPolling(); | |
| return; | |
| } | |
| if (process.hrtime(startTime)[0] > 30) { | |
| clearModuleGraphPolling(); | |
| const error = new Error( | |
| 'Timed out while waiting for the Vite module graph to initialize' | |
| ); | |
| logger.error('Failed to complete Vite change detection startup'); | |
| logger.error(error); | |
| notifyListenersOfStartupFailure({ | |
| type: 'unavailable', | |
| reason: error.message, | |
| error, | |
| }); | |
| return; | |
| } | |
| if (server.moduleGraph.fileToModulesMap.size > 0) { | |
| clearModuleGraphPolling(); | |
| await server.waitForRequestsIdle(); | |
| if (!watcherChangeHandler) { | |
| return; | |
| } | |
| server.watcher.on('all', watcherChangeHandler); | |
| watcherChangeHandler(); | |
| return; | |
| } | |
| waitForModuleGraph = setTimeout(() => { | |
| void pollForModuleGraph(); | |
| }, 1000); | |
| } catch (error) { | |
| clearModuleGraphPolling(); | |
| logger.error('Failed to complete Vite change detection startup'); | |
| logger.error(error instanceof Error ? error : String(error)); | |
| notifyListenersOfStartupFailure({ | |
| type: 'error', | |
| error: error instanceof Error ? error : new Error(String(error)), | |
| }); | |
| } | |
| }; | |
| waitForModuleGraph = setTimeout(() => { | |
| void pollForModuleGraph(); |
| const { mergeAlias } = await import('vite'); | ||
|
|
||
| const globalsList = Object.keys(externals); | ||
| const globalsCodeFilter = new RegExp(globalsList.map(escapeKeys).join('|')); |
There was a problem hiding this comment.
If externals is empty, new RegExp(globalsList.map(...).join('|')) becomes new RegExp(''), which matches all code and can cause the transform to run far more often than intended. Consider guarding for globalsList.length === 0 (e.g., skip registering the transform hook or use a non-matching regex) to avoid unnecessary work.
Suggested change
| const globalsCodeFilter = new RegExp(globalsList.map(escapeKeys).join('|')); | |
| const globalsCodeFilter = | |
| globalsList.length > 0 ? new RegExp(globalsList.map(escapeKeys).join('|')) : /$^/; |
Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com>
Contributor
Author
Merge conflicts resolved in 696302a. The only conflict was in the import section of |
Contributor
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds migration docs for React Native on-device addons moved to Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Automigrate Runner
participant Deps as Dependency Resolver
participant Updater as updateMainConfig helper
participant Config as mainConfig (configFile)
Runner->>Deps: getAllDependencies(project)
Deps-->>Runner: list (includes `@storybook/react-native`?)
alt `@storybook/react-native` present and ondevice addons present
Runner->>Config: read mainConfig.addons
Config-->>Runner: addons array (strings & objects)
Runner->>Updater: updateMainConfig({ mainConfigPath, dryRun }, callback)
Updater->>Config: removeEntryFromArray(['addons'], addon)
Updater->>Config: appendValueToArray(['deviceAddons'], addon)
Config-->>Updater: updated mainConfig
Updater-->>Runner: result
else no action needed
Runner-->>Runner: return null (no fix)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/addons/vitest/src/vitest-plugin/index.ts (1)
7-27:⚠️ Potential issue | 🟡 MinorRemove unused imports
loadPreviewOrConfigFileandisCsfFactoryPreview.Both imports are no longer used in this file. The CSF4 detection flow that relied on them was removed, so they should be deleted from the imports.
readConfig(line 25) is still used inextractTagsFromPreview, so that import is valid.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/addons/vitest/src/vitest-plugin/index.ts` around lines 7 - 27, Remove the two unused imports loadPreviewOrConfigFile and isCsfFactoryPreview from the top import lists (they appear in the first block with other storybook/internal/common imports and in the second block with csf-tools imports); leave all other imports (e.g., readConfig) intact so extractTagsFromPreview still works. Ensure you only delete those two identifiers from their respective import specifiers and keep formatting consistent.
🧹 Nitpick comments (2)
code/addons/pseudo-states/src/stories/CSSAtRules.stories.tsx (1)
3-7: Consider removing unused imports.
FORCE_REMOUNT,useChannel, anduseStoryContextappear to be unused after the refactor toPseudoStateGrid. If these were only needed by the removed JSX grid implementation, they can be cleaned up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/addons/pseudo-states/src/stories/CSSAtRules.stories.tsx` around lines 3 - 7, The imports FORCE_REMOUNT, useChannel, and useStoryContext are no longer used after refactoring to PseudoStateGrid; remove these unused imports from the top of CSSAtRules.stories.tsx (delete FORCE_REMOUNT from the storybook/internal import and remove useChannel and useStoryContext from the storybook/preview-api import) so only the necessary symbols (e.g., Meta, StoryObj, and PseudoStateGrid-related imports) remain and the file no longer contains dead imports.code/builders/builder-vite/src/index.test.ts (1)
317-327: Centralize these mock behaviors instead of overriding them inline in the tests.These cases reintroduce per-test mock implementations in the test body even though the suite already has shared setup in
beforeEach. A small helper/deferred fixture would keep the mock lifecycle consistent and easier to follow. As per coding guidelines, "Implement mock behaviors inbeforeEachblocks in Vitest tests" and "Avoid inline mock implementations within test cases in Vitest tests".Also applies to: 347-349, 370-370
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/builders/builder-vite/src/index.test.ts` around lines 317 - 327, The test currently overrides fakeViteServer.moduleGraph.fileToModulesMap and fakeViteServer.waitForRequestsIdle inline (using resolveIdle and createFileToModulesMap) — extract these into the shared beforeEach by adding a small helper (e.g., setupFakeViteServerMocks or createDeferredWaitForRequestsIdle) that sets fakeViteServer.moduleGraph.fileToModulesMap with createFileToModulesMap(['/src/Button.tsx', new Set([createViteModuleNode('/src/Button.tsx')])]) and assigns fakeViteServer.waitForRequestsIdle to a Deferred-style Promise whose resolver you expose (resolveIdle) so tests can trigger it; replace the inline mock implementations in tests (including the other occurrences that use waitForRequestsIdle and fileToModulesMap) to use that shared fixture instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Line 79: Update the ambiguous preview filename reference by replacing the
hardcoded string `.storybook/preview.ts` with a form that covers both TS and TSX
variants (e.g., `.storybook/preview.ts(x)`) in AGENTS.md so it clearly indicates
support for React-based frameworks; locate the occurrence of the literal
`.storybook/preview.ts` and change it to the parenthesized variant and ensure
surrounding wording still reads naturally.
In `@CHANGELOG.prerelease.md`:
- Line 26: Replace the truncated changelog entry that reads "Maintenance:
Extract getBuilderOptions helper across framewo… - [`#34260`]" with the full PR
title text; locate the line containing "getBuilderOptions" and PR "#34260" and
update it to the complete, non-truncated message so the release note displays
the entire title.
In `@code/addons/a11y/src/preview.test.tsx`:
- Around line 17-18: The Vitest mocks in preview.test.tsx use custom factories
and lack spy: true, violating the repo policy; change
vi.mock(import('./a11yRunner.ts')) to use vi.mock(import('./a11yRunner.ts'), {
spy: true }) and replace the factory-style vi.mock(import('./utils.ts'), async
(importOriginal) => { ... }) with vi.mock(import('./utils.ts'), { spy: true });
then move any mock implementations into the test file's beforeEach using
vi.mocked() to set return values/implementations for the a11yRunner and utils
mocks (e.g., configure the mocked functions there).
In `@code/addons/docs/docs/props-tables.md`:
- Line 93: The sentence ending in the docs line referencing customizing ArgTypes
for DocsPage and <ArgsTable story="xxx"> currently ends with a trailing comma;
change that comma to a period so the sentence reads as a complete statement
(ensure the text mentioning "DocsPage and <ArgsTable story=\"xxx\"> construct,
but not for the <ArgsTable of={component} /> construct" ends with a period).
In `@code/addons/vitest/src/custom-matchers.d.ts`:
- Line 3: Update the relative type import on the top of custom-matchers.d.ts to
include the explicit .ts extension: change the import of
LiveRegionMatcherOptions from '../../../core/src/shared/utils/toHaveLiveRegion'
to the same path with a .ts suffix so the TypeScript import-extension rule is
satisfied and the type resolves consistently.
In `@code/addons/vitest/src/node/boot-test-runner.test.ts`:
- Line 10: The test imports the logger using "../logger.ts" but the vi.mock call
targets "../logger", so Vitest may not match and the mock won't apply; update
the vi.mock target to "../logger.ts" (the same specifier used in the import) so
the mock for the module referenced by the import is applied (look for the
vi.mock call around line 56 and change its module specifier to "../logger.ts" to
match the import { log } from '../logger.ts').
In `@code/addons/vitest/src/use-test-provider-state.ts`:
- Line 51: The code unguardedly indexes statusValueToStoryIdsMap with
status.value as TestStatusValue which can throw if status.value is unexpected;
before pushing status.storyId, add a runtime guard that checks whether
status.value is a valid key (e.g., typeof status.value === 'string' and
status.value in statusValueToStoryIdsMap or using
Object.prototype.hasOwnProperty.call(statusValueToStoryIdsMap, status.value))
and only then call statusValueToStoryIdsMap[status.value].push(status.storyId);
otherwise handle the unknown status safely (e.g., ignore, log, or add to a
fallback bucket) to prevent runtime crashes.
In `@code/builders/builder-vite/src/index.test.ts`:
- Around line 10-11: The test mock specifier doesn't match the import: update
the vitest mock call for the vite server so it uses the exact module specifier
used by the import (change vi.mock('./vite-server', { spy: true }) to the exact
'./vite-server.ts' specifier) so the createViteServer import in index.test.ts is
properly mocked; keep the { spy: true } option and ensure any other mocks follow
the same exact-specifier rule.
In `@code/builders/builder-vite/src/plugins/vite-inject-mocker/plugin.ts`:
- Around line 44-52: The current object-form resolveId hook (resolveId: {
filter, handler }) is incompatible with Vite <6.3 and must be replaced with the
function form so ENTRY_PATH is resolved across supported Vite versions; update
the resolveId export in plugin.ts to a function-style hook named
resolveId(source) that checks if source === ENTRY_PATH and returns
mockerRuntimePath when matched (otherwise returns undefined), ensuring the
module resolution logic using ENTRY_PATH and mockerRuntimePath continues to work
in Vite 5.x and 6.0–6.2.x.
In `@code/builders/builder-vite/src/utils/build-module-graph.ts`:
- Around line 10-21: getModuleFileFromMap currently scans fileToModulesMap
entries O(N) for each lookup; precompute a reverse lookup by creating a
WeakMap<ViteModuleNode,string> (e.g., moduleNodeToFile) when fileToModulesMap is
built/updated and populate it by iterating fileToModulesMap once, then replace
the loop inside getModuleFileFromMap with a constant-time
moduleNodeToFile.get(viteModuleNode) lookup; also update the other similar
lookup site (the function/usage around the other occurrence noted) to use the
same WeakMap and ensure the WeakMap stays in scope and is updated whenever
fileToModulesMap changes.
In `@code/core/custom-matchers.d.ts`:
- Line 3: The import for LiveRegionMatcherOptions in custom-matchers.d.ts uses
an incorrect cross-package path; update the import to point to the package-local
shared utils module with an explicit .ts extension (i.e., import
LiveRegionMatcherOptions from the local shared/utils/toHaveLiveRegion.ts) so the
type resolves within the same package and conforms to the explicit-extension
guideline.
In `@code/core/src/backgrounds/types.ts`:
- Line 1: The import of PARAM_KEY is currently type-only but it's used at
runtime as a computed property key (e.g. [PARAM_KEY]) so change the import from
a type-only import to a normal value import; replace "import type { PARAM_KEY }"
with a value import for PARAM_KEY in types.ts so the symbol is available at
runtime where it's referenced (see usages of PARAM_KEY around the computed
property access).
In `@code/core/src/common/js-package-manager/Yarn2Proxy.test.ts`:
- Around line 5-7: The test imports executeCommand from '../utils/command.ts'
but the module mock still targets '../utils/command' so the mock doesn't match;
update the mock specifier in Yarn2Proxy.test.ts to use the explicit extension
(change any mock/spy import of '../utils/command' to '../utils/command.ts') so
that the mocked executeCommand used by the Yarn2Proxy tests resolves correctly.
In `@code/core/src/common/presets.test.ts`:
- Around line 8-9: The mock target string used in the vitest.mock call doesn't
include the `.ts` extension so it doesn't match the actual import and the mock
of resolveUtils is not applied; update the module string in the mock to include
the `.ts` extension to match the import (so tests that call
mockedResolveUtils.importModule.mockImplementation(...) will be intercepted),
then run the tests to confirm the mocked implementations are used.
---
Outside diff comments:
In `@code/addons/vitest/src/vitest-plugin/index.ts`:
- Around line 7-27: Remove the two unused imports loadPreviewOrConfigFile and
isCsfFactoryPreview from the top import lists (they appear in the first block
with other storybook/internal/common imports and in the second block with
csf-tools imports); leave all other imports (e.g., readConfig) intact so
extractTagsFromPreview still works. Ensure you only delete those two identifiers
from their respective import specifiers and keep formatting consistent.
---
Nitpick comments:
In `@code/addons/pseudo-states/src/stories/CSSAtRules.stories.tsx`:
- Around line 3-7: The imports FORCE_REMOUNT, useChannel, and useStoryContext
are no longer used after refactoring to PseudoStateGrid; remove these unused
imports from the top of CSSAtRules.stories.tsx (delete FORCE_REMOUNT from the
storybook/internal import and remove useChannel and useStoryContext from the
storybook/preview-api import) so only the necessary symbols (e.g., Meta,
StoryObj, and PseudoStateGrid-related imports) remain and the file no longer
contains dead imports.
In `@code/builders/builder-vite/src/index.test.ts`:
- Around line 317-327: The test currently overrides
fakeViteServer.moduleGraph.fileToModulesMap and
fakeViteServer.waitForRequestsIdle inline (using resolveIdle and
createFileToModulesMap) — extract these into the shared beforeEach by adding a
small helper (e.g., setupFakeViteServerMocks or
createDeferredWaitForRequestsIdle) that sets
fakeViteServer.moduleGraph.fileToModulesMap with
createFileToModulesMap(['/src/Button.tsx', new
Set([createViteModuleNode('/src/Button.tsx')])]) and assigns
fakeViteServer.waitForRequestsIdle to a Deferred-style Promise whose resolver
you expose (resolveIdle) so tests can trigger it; replace the inline mock
implementations in tests (including the other occurrences that use
waitForRequestsIdle and fileToModulesMap) to use that shared fixture instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 01c3d895-b57a-46c6-b57c-c4c019a9a09f
📒 Files selected for processing (300)
.oxfmtrc.json.vscode/settings.jsonAGENTS.mdCHANGELOG.mdCHANGELOG.prerelease.mdMIGRATION.mdcode/.eslintrc.jscode/.storybook/bench/bench.stories.tsxcode/.storybook/main.tscode/.storybook/preview.tsxcode/.storybook/storybook.setup.tscode/addons/a11y/build-config.tscode/addons/a11y/package.jsoncode/addons/a11y/src/a11yRunner.test.tscode/addons/a11y/src/a11yRunner.tscode/addons/a11y/src/a11yRunnerUtils.test.tscode/addons/a11y/src/a11yRunnerUtils.tscode/addons/a11y/src/axeRuleMappingHelper.tscode/addons/a11y/src/components/A11YPanel.stories.tsxcode/addons/a11y/src/components/A11YPanel.test.tsxcode/addons/a11y/src/components/A11YPanel.tsxcode/addons/a11y/src/components/A11yContext.test.tsxcode/addons/a11y/src/components/A11yContext.tsxcode/addons/a11y/src/components/Report/Details.tsxcode/addons/a11y/src/components/Report/Report.stories.tsxcode/addons/a11y/src/components/Report/Report.tsxcode/addons/a11y/src/components/Tabs.tsxcode/addons/a11y/src/components/TestDiscrepancyMessage.stories.tsxcode/addons/a11y/src/components/TestDiscrepancyMessage.tsxcode/addons/a11y/src/components/VisionSimulator.stories.tsxcode/addons/a11y/src/components/VisionSimulator.tsxcode/addons/a11y/src/index.tscode/addons/a11y/src/manager.test.tsxcode/addons/a11y/src/manager.tsxcode/addons/a11y/src/postinstall.tscode/addons/a11y/src/preview.test.tsxcode/addons/a11y/src/preview.tsxcode/addons/a11y/src/types.tscode/addons/a11y/src/withVisionSimulator.tscode/addons/a11y/vitest.config.tscode/addons/docs/docs/props-tables.mdcode/addons/docs/package.jsoncode/addons/docs/src/mdx-plugin.tscode/addons/links/build-config.tscode/addons/links/package.jsoncode/addons/links/src/index.tscode/addons/links/src/manager.tscode/addons/links/src/preview.tscode/addons/links/src/react/components/link.test.tsxcode/addons/links/src/react/components/link.tsxcode/addons/links/src/react/index.tscode/addons/links/src/utils.test.tscode/addons/links/src/utils.tscode/addons/links/vitest.config.tscode/addons/onboarding/build-config.tscode/addons/onboarding/example-stories/Button.stories.tsxcode/addons/onboarding/package.jsoncode/addons/onboarding/src/Onboarding.tsxcode/addons/onboarding/src/Survey.tsxcode/addons/onboarding/src/components/Confetti/Confetti.stories.tsxcode/addons/onboarding/src/components/List/List.stories.tsxcode/addons/onboarding/src/components/List/List.tsxcode/addons/onboarding/src/components/List/ListItem/ListItem.tsxcode/addons/onboarding/src/features/IntentSurvey/IntentSurvey.stories.tsxcode/addons/onboarding/src/features/IntentSurvey/IntentSurvey.tsxcode/addons/onboarding/src/features/SplashScreen/SplashScreen.stories.tsxcode/addons/onboarding/src/manager.tsxcode/addons/onboarding/src/preset.tscode/addons/onboarding/vitest.config.tscode/addons/pseudo-states/build-config.tscode/addons/pseudo-states/package.jsoncode/addons/pseudo-states/src/index.tscode/addons/pseudo-states/src/manager.tscode/addons/pseudo-states/src/manager/PseudoStateTool.tsxcode/addons/pseudo-states/src/preview.tscode/addons/pseudo-states/src/preview/rewriteStyleSheet.test.tscode/addons/pseudo-states/src/preview/rewriteStyleSheet.tscode/addons/pseudo-states/src/preview/splitSelectors.test.tscode/addons/pseudo-states/src/preview/withPseudoState.tscode/addons/pseudo-states/src/stories/Button.stories.tsxcode/addons/pseudo-states/src/stories/CSSAtRules.stories.tsxcode/addons/pseudo-states/src/stories/CustomElement.stories.tsxcode/addons/pseudo-states/src/stories/CustomElementNested.stories.tsxcode/addons/pseudo-states/src/stories/CustomElementNested.tsxcode/addons/pseudo-states/src/stories/Input.stories.tsxcode/addons/pseudo-states/src/stories/NestedRules.stories.tsxcode/addons/pseudo-states/src/stories/Portal.stories.tsxcode/addons/pseudo-states/src/stories/PseudoStateGrid.tsxcode/addons/pseudo-states/src/stories/ShadowRoot.stories.tsxcode/addons/pseudo-states/src/stories/ShadowRootWithPart.stories.tsxcode/addons/pseudo-states/src/types.test-d.tscode/addons/pseudo-states/src/types.tscode/addons/pseudo-states/vitest.config.tscode/addons/themes/build-config.tscode/addons/themes/package.jsoncode/addons/themes/src/constants.tscode/addons/themes/src/decorators/class-name.decorator.tsxcode/addons/themes/src/decorators/data-attribute.decorator.tsxcode/addons/themes/src/decorators/helpers.tscode/addons/themes/src/decorators/index.tscode/addons/themes/src/decorators/provider.decorator.tsxcode/addons/themes/src/index.tscode/addons/themes/src/manager.tsxcode/addons/themes/src/preview.tscode/addons/themes/src/theme-switcher.tsxcode/addons/themes/vitest.config.tscode/addons/vitest/build-config.tscode/addons/vitest/package.jsoncode/addons/vitest/src/components/Description.tsxcode/addons/vitest/src/components/GlobalErrorModal.stories.tsxcode/addons/vitest/src/components/GlobalErrorModal.tsxcode/addons/vitest/src/components/RelativeTime.stories.tsxcode/addons/vitest/src/components/SidebarContextMenu.tsxcode/addons/vitest/src/components/TestProviderRender.stories.tsxcode/addons/vitest/src/components/TestProviderRender.tsxcode/addons/vitest/src/components/TestStatusIcon.stories.tsxcode/addons/vitest/src/constants.tscode/addons/vitest/src/custom-matchers.d.tscode/addons/vitest/src/logger.tscode/addons/vitest/src/manager-store.mock.tscode/addons/vitest/src/manager-store.tscode/addons/vitest/src/manager.tsxcode/addons/vitest/src/node/boot-test-runner.test.tscode/addons/vitest/src/node/boot-test-runner.tscode/addons/vitest/src/node/coverage-reporter.tscode/addons/vitest/src/node/reporter.tscode/addons/vitest/src/node/test-manager.test.tscode/addons/vitest/src/node/test-manager.tscode/addons/vitest/src/node/vitest-manager.tscode/addons/vitest/src/node/vitest.tscode/addons/vitest/src/postinstall.test.tscode/addons/vitest/src/postinstall.tscode/addons/vitest/src/preset.tscode/addons/vitest/src/updateVitestFile.config.3.2.test.tscode/addons/vitest/src/updateVitestFile.config.4.test.tscode/addons/vitest/src/updateVitestFile.config.test.tscode/addons/vitest/src/updateVitestFile.config.workspace.test.tscode/addons/vitest/src/updateVitestFile.test.tscode/addons/vitest/src/updateVitestFile.tscode/addons/vitest/src/use-test-provider-state.tscode/addons/vitest/src/utils.tscode/addons/vitest/src/vitest-plugin/index.tscode/addons/vitest/src/vitest-plugin/setup-file.test.tscode/addons/vitest/src/vitest-plugin/setup-file.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/addons/vitest/src/vitest-plugin/utils.tscode/addons/vitest/src/vitest-plugin/viewports.test.tscode/addons/vitest/vitest.config.tscode/builders/builder-vite/build-config.tscode/builders/builder-vite/package.jsoncode/builders/builder-vite/src/build.tscode/builders/builder-vite/src/codegen-importfn-script.test.tscode/builders/builder-vite/src/codegen-importfn-script.tscode/builders/builder-vite/src/codegen-modern-iframe-script.test.tscode/builders/builder-vite/src/codegen-modern-iframe-script.tscode/builders/builder-vite/src/codegen-project-annotations.tscode/builders/builder-vite/src/index.test.tscode/builders/builder-vite/src/index.tscode/builders/builder-vite/src/plugins/code-generator-plugin.tscode/builders/builder-vite/src/plugins/index.tscode/builders/builder-vite/src/plugins/inject-export-order-plugin.tscode/builders/builder-vite/src/plugins/storybook-entry-plugin.tscode/builders/builder-vite/src/plugins/storybook-external-globals-plugin.test.tscode/builders/builder-vite/src/plugins/storybook-external-globals-plugin.tscode/builders/builder-vite/src/plugins/storybook-optimize-deps-plugin.test.tscode/builders/builder-vite/src/plugins/storybook-optimize-deps-plugin.tscode/builders/builder-vite/src/plugins/storybook-project-annotations-plugin.tscode/builders/builder-vite/src/plugins/storybook-runtime-plugin.tscode/builders/builder-vite/src/plugins/strip-story-hmr-boundaries.tscode/builders/builder-vite/src/plugins/vite-inject-mocker/plugin.tscode/builders/builder-vite/src/plugins/vite-mock/plugin.tscode/builders/builder-vite/src/plugins/webpack-stats-plugin.tscode/builders/builder-vite/src/preset.tscode/builders/builder-vite/src/transform-iframe-html.tscode/builders/builder-vite/src/utils/build-module-graph.test.tscode/builders/builder-vite/src/utils/build-module-graph.tscode/builders/builder-vite/src/utils/has-vite-plugins.test.tscode/builders/builder-vite/src/utils/process-preview-annotation.test.tscode/builders/builder-vite/src/utils/without-vite-plugins.test.tscode/builders/builder-vite/src/vite-config.test.tscode/builders/builder-vite/src/vite-config.tscode/builders/builder-vite/src/vite-server.tscode/builders/builder-vite/vitest.config.tscode/builders/builder-webpack5/build-config.tscode/builders/builder-webpack5/package.jsoncode/builders/builder-webpack5/src/index.tscode/builders/builder-webpack5/src/presets/custom-webpack-preset.tscode/builders/builder-webpack5/src/presets/preview-preset.tscode/builders/builder-webpack5/src/preview/iframe-webpack.config.tscode/builders/builder-webpack5/src/preview/virtual-module-mapping.tscode/builders/builder-webpack5/vitest.config.tscode/core/build-config.tscode/core/custom-matchers.d.tscode/core/package.jsoncode/core/scripts/generate-source-files.tscode/core/src/__tests/preview-errors.test.tscode/core/src/__tests/server-errors.test.tscode/core/src/__tests/storybook-error.test.tscode/core/src/actions/addArgs.tscode/core/src/actions/addArgsHelpers.test.tscode/core/src/actions/addArgsHelpers.tscode/core/src/actions/components/ActionLogger/index.tsxcode/core/src/actions/components/Title.tsxcode/core/src/actions/containers/ActionLogger/ActionLogger.stories.tsxcode/core/src/actions/containers/ActionLogger/index.tsxcode/core/src/actions/decorator.tscode/core/src/actions/index.tscode/core/src/actions/loaders.tscode/core/src/actions/manager.tsxcode/core/src/actions/models/ActionDisplay.tscode/core/src/actions/models/ActionsFunction.tscode/core/src/actions/models/ActionsMap.tscode/core/src/actions/models/HandlerFunction.test-d.tscode/core/src/actions/models/index.tscode/core/src/actions/preview.tscode/core/src/actions/runtime/action.tscode/core/src/actions/runtime/actions.tscode/core/src/actions/runtime/configureActions.tscode/core/src/actions/runtime/index.tscode/core/src/babel/expression-resolver.test.tscode/core/src/babel/index.tscode/core/src/babel/vitest-config-helpers.test.tscode/core/src/babel/vitest-config-helpers.tscode/core/src/backgrounds/components/Tool.tsxcode/core/src/backgrounds/decorator.tscode/core/src/backgrounds/defaults.tscode/core/src/backgrounds/manager.tsxcode/core/src/backgrounds/preview.tscode/core/src/backgrounds/types.tscode/core/src/bin/core.tscode/core/src/bin/dispatcher.tscode/core/src/bin/loader.test.tscode/core/src/bin/loader.tscode/core/src/builder-manager/index.tscode/core/src/builder-manager/utils/data.tscode/core/src/builder-manager/utils/files.test.tscode/core/src/builder-manager/utils/files.tscode/core/src/builder-manager/utils/template.tscode/core/src/channels/index.test.tscode/core/src/channels/index.tscode/core/src/channels/main.tscode/core/src/channels/postmessage/index.tscode/core/src/channels/websocket/index.tscode/core/src/cli/AddonVitestService.constants.tscode/core/src/cli/AddonVitestService.test.tscode/core/src/cli/AddonVitestService.tscode/core/src/cli/NpmOptions.tscode/core/src/cli/dirs.tscode/core/src/cli/eslintPlugin.test.tscode/core/src/cli/eslintPlugin.tscode/core/src/cli/globalSettings.test.tscode/core/src/cli/globalSettings.tscode/core/src/cli/helpers.test.tscode/core/src/cli/helpers.tscode/core/src/cli/index.tscode/core/src/client-logger/index.test.tscode/core/src/common/config.test.tscode/core/src/common/index.tscode/core/src/common/js-package-manager/BUNProxy.tscode/core/src/common/js-package-manager/JsPackageManager.test.tscode/core/src/common/js-package-manager/JsPackageManager.tscode/core/src/common/js-package-manager/JsPackageManagerFactory.test.tscode/core/src/common/js-package-manager/JsPackageManagerFactory.tscode/core/src/common/js-package-manager/NPMProxy.test.tscode/core/src/common/js-package-manager/NPMProxy.tscode/core/src/common/js-package-manager/PNPMProxy.test.tscode/core/src/common/js-package-manager/PNPMProxy.tscode/core/src/common/js-package-manager/Yarn1Proxy.test.tscode/core/src/common/js-package-manager/Yarn1Proxy.tscode/core/src/common/js-package-manager/Yarn2Proxy.test.tscode/core/src/common/js-package-manager/Yarn2Proxy.tscode/core/src/common/js-package-manager/index.tscode/core/src/common/presets.test.tscode/core/src/common/presets.tscode/core/src/common/utils/__tests__/interpret-files.test.tscode/core/src/common/utils/__tests__/normalize-stories.test.tscode/core/src/common/utils/__tests__/paths.test.tscode/core/src/common/utils/__tests__/template.test.tscode/core/src/common/utils/cache.tscode/core/src/common/utils/cli.test.tscode/core/src/common/utils/cli.tscode/core/src/common/utils/envs.tscode/core/src/common/utils/file-cache.tscode/core/src/common/utils/formatter.test.tscode/core/src/common/utils/get-addon-annotations.test.tscode/core/src/common/utils/get-addon-annotations.tscode/core/src/common/utils/get-addon-names.test.tscode/core/src/common/utils/get-addon-names.tscode/core/src/common/utils/get-framework-name.test.tscode/core/src/common/utils/get-framework-name.tscode/core/src/common/utils/get-renderer-name.test.tscode/core/src/common/utils/get-renderer-name.tscode/core/src/common/utils/get-story-id.test.tscode/core/src/common/utils/get-story-id.tscode/core/src/common/utils/get-storybook-configuration.test.tscode/core/src/common/utils/get-storybook-info.tscode/core/src/common/utils/get-storybook-refs.test.tscode/core/src/common/utils/get-storybook-refs.tscode/core/src/common/utils/interpret-require.tscode/core/src/common/utils/load-main-config.ts
|
|
||
| - `.storybook/main.ts` is loaded at startup | ||
| - `.storybook/preview.ts` is bundled into preview | ||
| - `.storybook/preview.ts` is bundled into preview (TSX for React-based frameworks) |
Contributor
There was a problem hiding this comment.
Clarify preview filename wording to avoid ambiguity.
The line currently hardcodes .storybook/preview.ts but also mentions TSX. Consider using .storybook/preview.ts(x) to avoid conflicting guidance for React-based frameworks.
Based on learnings: Applies to AGENTS.md : Keep AGENTS.md up to date when Storybook's architecture, tooling, workflows, or contributor guidance changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` at line 79, Update the ambiguous preview filename reference by
replacing the hardcoded string `.storybook/preview.ts` with a form that covers
both TS and TSX variants (e.g., `.storybook/preview.ts(x)`) in AGENTS.md so it
clearly indicates support for React-based frameworks; locate the occurrence of
the literal `.storybook/preview.ts` and change it to the parenthesized variant
and ensure surrounding wording still reads naturally.
| - CLI: Prompt for init crash reports - [#34316](https://github.com/storybookjs/storybook/pull/34316), thanks @JReinhold! | ||
| - CSF4: Fix duplicate preview loading issue in Vitest - [#34361](https://github.com/storybookjs/storybook/pull/34361), thanks @valentinpalkovic! | ||
| - Maintenance: Add assertions outside step incorrectly nested in interactions panel - [#34296](https://github.com/storybookjs/storybook/pull/34296), thanks @majiayu000! | ||
| - Maintenance: Extract getBuilderOptions helper across framewo… - [#34260](https://github.com/storybookjs/storybook/pull/34260), thanks @alex-js-ltd! |
Contributor
There was a problem hiding this comment.
Fix truncated changelog title text.
Line 26 appears cut off (...across framewo…), which makes the release note ambiguous. Please use the full PR title text.
✏️ Suggested edit
-- Maintenance: Extract getBuilderOptions helper across framewo… - [`#34260`](https://github.com/storybookjs/storybook/pull/34260), thanks `@alex-js-ltd`!
+- Maintenance: Extract getBuilderOptions helper across frameworks - [`#34260`](https://github.com/storybookjs/storybook/pull/34260), thanks `@alex-js-ltd`!📝 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.
Suggested change
| - Maintenance: Extract getBuilderOptions helper across framewo… - [#34260](https://github.com/storybookjs/storybook/pull/34260), thanks @alex-js-ltd! | |
| - Maintenance: Extract getBuilderOptions helper across frameworks - [`#34260`](https://github.com/storybookjs/storybook/pull/34260), thanks `@alex-js-ltd`! |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.prerelease.md` at line 26, Replace the truncated changelog entry
that reads "Maintenance: Extract getBuilderOptions helper across framewo… -
[`#34260`]" with the full PR title text; locate the line containing
"getBuilderOptions" and PR "#34260" and update it to the complete, non-truncated
message so the release note displays the entire title.
Comment on lines
+17
to
+18
| vi.mock(import('./a11yRunner.ts')); | ||
| vi.mock(import('./utils.ts'), async (importOriginal) => { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n code/addons/a11y/src/preview.test.tsx | head -100Repository: storybookjs/storybook
Length of output: 3854
🏁 Script executed:
cat code/.cursor/rules/spy-mocking.mdcRepository: storybookjs/storybook
Length of output: 131
🏁 Script executed:
find . -name "spy-mocking.mdc" -o -name "*spy*mocking*" 2>/dev/nullRepository: storybookjs/storybook
Length of output: 96
🏁 Script executed:
cat .cursor/rules/spy-mocking.mdcRepository: storybookjs/storybook
Length of output: 1585
🏁 Script executed:
# Search for other test files using vi.mock with custom factories to understand the pattern
rg 'vi\.mock.*async.*importOriginal' --type ts --type tsx -A 5 -B 2 | head -100Repository: storybookjs/storybook
Length of output: 92
🏁 Script executed:
rg 'vi\.mock.*async.*importOriginal' -A 5 -B 2 | head -150Repository: storybookjs/storybook
Length of output: 12324
🏁 Script executed:
rg 'vi\.mock.*spy.*true' | head -50Repository: storybookjs/storybook
Length of output: 5570
🏁 Script executed:
rg 'vi\.mock.*spy.*true.*async' | head -20Repository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
# Check if any test files use spy: true with a factory function (both combined)
rg "spy:\s*true.*async\s*\(" | head -20Repository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
# Check for vi.mock with both spy: true and custom factory implementations
rg -A 10 "vi\.mock.*spy:\s*true" | grep -A 10 "async\|importOriginal" | head -50Repository: storybookjs/storybook
Length of output: 4394
🏁 Script executed:
# Check the Vitest documentation pattern in comments or look for any examples mixing spy: true with factory
rg "spy.*true.*\(" -B 5 -A 10 | grep -A 15 "factory\|importOriginal" | head -40Repository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
# Check the actual content around the preview.test.tsx file more thoroughly to understand the context
cat -n code/addons/a11y/src/preview.test.tsx | head -150Repository: storybookjs/storybook
Length of output: 5492
Use spy: true for these Vitest mocks.
Lines 17-18 don't include { spy: true }, which conflicts with the repository's Vitest mock policy. Per the coding guidelines, all package and file mocks must use { spy: true } with mock behaviors implemented in beforeEach blocks.
For line 17, add { spy: true }. For lines 18-25, replace the custom factory with { spy: true } and move the mock overrides to beforeEach using vi.mocked():
Suggested adjustment
-vi.mock(import('./a11yRunner.ts'));
+vi.mock(import('./a11yRunner.ts'), { spy: true });
-vi.mock(import('./utils.ts'), async (importOriginal) => {
- const mod = await importOriginal(); // type is inferred
- return {
- ...mod,
- getIsVitestRunning: mocks.getIsVitestRunning,
- getIsVitestStandaloneRun: mocks.getIsVitestStandaloneRun,
- };
-});
+vi.mock(import('./utils.ts'), { spy: true });Then in the beforeEach block, configure the mocks:
beforeEach(() => {
vi.mocked(getIsVitestRunning).mockReturnValue(false);
vi.mocked(getIsVitestStandaloneRun).mockReturnValue(true);
});📝 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.
Suggested change
| vi.mock(import('./a11yRunner.ts')); | |
| vi.mock(import('./utils.ts'), async (importOriginal) => { | |
| vi.mock(import('./a11yRunner.ts'), { spy: true }); | |
| vi.mock(import('./utils.ts'), { spy: true }); | |
| beforeEach(() => { | |
| vi.mocked(getIsVitestRunning).mockReturnValue(false); | |
| vi.mocked(getIsVitestStandaloneRun).mockReturnValue(true); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/addons/a11y/src/preview.test.tsx` around lines 17 - 18, The Vitest mocks
in preview.test.tsx use custom factories and lack spy: true, violating the repo
policy; change vi.mock(import('./a11yRunner.ts')) to use
vi.mock(import('./a11yRunner.ts'), { spy: true }) and replace the factory-style
vi.mock(import('./utils.ts'), async (importOriginal) => { ... }) with
vi.mock(import('./utils.ts'), { spy: true }); then move any mock implementations
into the test file's beforeEach using vi.mocked() to set return
values/implementations for the a11yRunner and utils mocks (e.g., configure the
mocked functions there).
| Props tables are rendered from an internal data structure called `ArgTypes`. When you declare a story's `component` metadata, Docs automatically extracts `ArgTypes` based on the component's properties. | ||
|
|
||
| You can can customize what's shown in the props table by [customizing the `ArgTypes` data](#customizing-argtypes). This is currently available for `DocsPage` and `<ArgsTable story="xxx">` construct, but not for the `<ArgsTable of={component} />` construct, | ||
| You can customize what's shown in the props table by [customizing the `ArgTypes` data](#customizing-argtypes). This is currently available for `DocsPage` and `<ArgsTable story="xxx">` construct, but not for the `<ArgsTable of={component} />` construct, |
Contributor
There was a problem hiding this comment.
Replace trailing comma with a period in this sentence.
Line 93 ends with a comma, which reads as an incomplete sentence in user-facing docs.
✍️ Proposed fix
-You can customize what's shown in the props table by [customizing the `ArgTypes` data](`#customizing-argtypes`). This is currently available for `DocsPage` and `<ArgsTable story="xxx">` construct, but not for the `<ArgsTable of={component} />` construct,
+You can customize what's shown in the props table by [customizing the `ArgTypes` data](`#customizing-argtypes`). This is currently available for `DocsPage` and `<ArgsTable story="xxx">` construct, but not for the `<ArgsTable of={component} />` construct.📝 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.
Suggested change
| You can customize what's shown in the props table by [customizing the `ArgTypes` data](#customizing-argtypes). This is currently available for `DocsPage` and `<ArgsTable story="xxx">` construct, but not for the `<ArgsTable of={component} />` construct, | |
| You can customize what's shown in the props table by [customizing the `ArgTypes` data](`#customizing-argtypes`). This is currently available for `DocsPage` and `<ArgsTable story="xxx">` construct, but not for the `<ArgsTable of={component} />` construct. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/addons/docs/docs/props-tables.md` at line 93, The sentence ending in the
docs line referencing customizing ArgTypes for DocsPage and <ArgsTable
story="xxx"> currently ends with a trailing comma; change that comma to a period
so the sentence reads as a complete statement (ensure the text mentioning
"DocsPage and <ArgsTable story=\"xxx\"> construct, but not for the <ArgsTable
of={component} /> construct" ends with a period).
| @@ -0,0 +1,9 @@ | |||
| import 'storybook/test'; | |||
|
|
|||
| import type { LiveRegionMatcherOptions } from '../../../core/src/shared/utils/toHaveLiveRegion'; | |||
Contributor
There was a problem hiding this comment.
Add explicit extension for the relative type import.
Line 3 is missing the .ts suffix, which makes this file inconsistent with the repo’s TypeScript import-extension rule.
♻️ Proposed fix
-import type { LiveRegionMatcherOptions } from '../../../core/src/shared/utils/toHaveLiveRegion';
+import type { LiveRegionMatcherOptions } from '../../../core/src/shared/utils/toHaveLiveRegion.ts';As per coding guidelines **/*.{ts,tsx,js,jsx}: Prefer explicit file extensions for relative code imports and exports in TypeScript source (e.g., ./foo.ts or ./bar.tsx) when the target is another TS/JS module in the repository.
📝 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.
Suggested change
| import type { LiveRegionMatcherOptions } from '../../../core/src/shared/utils/toHaveLiveRegion'; | |
| import type { LiveRegionMatcherOptions } from '../../../core/src/shared/utils/toHaveLiveRegion.ts'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/addons/vitest/src/custom-matchers.d.ts` at line 3, Update the relative
type import on the top of custom-matchers.d.ts to include the explicit .ts
extension: change the import of LiveRegionMatcherOptions from
'../../../core/src/shared/utils/toHaveLiveRegion' to the same path with a .ts
suffix so the TypeScript import-extension rule is satisfied and the type
resolves consistently.
Comment on lines
+10
to
+21
| const getModuleFileFromMap = (viteModuleNode: { | ||
| file: string | null; | ||
| type: ViteModuleNode['type']; | ||
| importers: Set<ViteModuleNode>; | ||
| importedModules: Set<ViteModuleNode>; | ||
| }): string | undefined => { | ||
| for (const [filePath, viteModuleSet] of fileToModulesMap.entries()) { | ||
| if (viteModuleSet.has(viteModuleNode as ViteModuleNode)) { | ||
| return filePath; | ||
| } | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
Precompute node→file lookup to avoid repeated full scans.
getModuleFileFromMap does an O(N) scan and is invoked inside nested importer/imported traversals, which can become very expensive on large graphs. Build a reverse WeakMap<ViteModuleNode, string> once, then do O(1) lookups.
⚡ Suggested optimization sketch
export function buildModuleGraph(
fileToModulesMap: ViteDevServer['moduleGraph']['fileToModulesMap']
): ModuleGraph {
const moduleGraph: ModuleGraph = new Map();
const moduleNodeMap = new WeakMap<object, ModuleNode>();
+ const fileByViteNode = new WeakMap<ViteModuleNode, string>();
+
+ for (const [filePath, viteModuleSet] of fileToModulesMap.entries()) {
+ for (const viteModuleNode of viteModuleSet) {
+ fileByViteNode.set(viteModuleNode, filePath);
+ }
+ }
- const getModuleFileFromMap = (viteModuleNode: { ... }): string | undefined => {
- for (const [filePath, viteModuleSet] of fileToModulesMap.entries()) {
- if (viteModuleSet.has(viteModuleNode as ViteModuleNode)) {
- return filePath;
- }
- }
- };
+ const getModuleFileFromMap = (viteModuleNode: ViteModuleNode): string | undefined =>
+ fileByViteNode.get(viteModuleNode);Also applies to: 61-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/builders/builder-vite/src/utils/build-module-graph.ts` around lines 10 -
21, getModuleFileFromMap currently scans fileToModulesMap entries O(N) for each
lookup; precompute a reverse lookup by creating a WeakMap<ViteModuleNode,string>
(e.g., moduleNodeToFile) when fileToModulesMap is built/updated and populate it
by iterating fileToModulesMap once, then replace the loop inside
getModuleFileFromMap with a constant-time moduleNodeToFile.get(viteModuleNode)
lookup; also update the other similar lookup site (the function/usage around the
other occurrence noted) to use the same WeakMap and ensure the WeakMap stays in
scope and is updated whenever fileToModulesMap changes.
| @@ -0,0 +1,9 @@ | |||
| import 'storybook/test'; | |||
|
|
|||
| import type { LiveRegionMatcherOptions } from '../../../core/src/shared/utils/toHaveLiveRegion'; | |||
Contributor
There was a problem hiding this comment.
Fix the relative import path and add explicit .ts extension.
The import path '../../../core/src/shared/utils/toHaveLiveRegion' appears incorrect for a file located at code/core/custom-matchers.d.ts. It should be a direct relative path within the same package. Additionally, per coding guidelines, use explicit .ts extensions for relative imports.
Proposed fix
-import type { LiveRegionMatcherOptions } from '../../../core/src/shared/utils/toHaveLiveRegion';
+import type { LiveRegionMatcherOptions } from './src/shared/utils/toHaveLiveRegion.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.
Suggested change
| import type { LiveRegionMatcherOptions } from '../../../core/src/shared/utils/toHaveLiveRegion'; | |
| import type { LiveRegionMatcherOptions } from './src/shared/utils/toHaveLiveRegion.ts'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/core/custom-matchers.d.ts` at line 3, The import for
LiveRegionMatcherOptions in custom-matchers.d.ts uses an incorrect cross-package
path; update the import to point to the package-local shared utils module with
an explicit .ts extension (i.e., import LiveRegionMatcherOptions from the local
shared/utils/toHaveLiveRegion.ts) so the type resolves within the same package
and conforms to the explicit-extension guideline.
| @@ -1,4 +1,4 @@ | |||
| import type { PARAM_KEY } from './constants'; | |||
| import type { PARAM_KEY } from './constants.ts'; | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "export\\s+(const|let|var|type)\\s+PARAM_KEY" code/core/src/backgrounds/constants.ts
rg -n "import\\s+type\\s+\\{\\s*PARAM_KEY\\s*\\}\\s+from\\s+'\\./constants\\.ts'" code/core/src/backgrounds/types.ts
rg -n "\\[PARAM_KEY\\]" code/core/src/backgrounds/types.tsRepository: storybookjs/storybook
Length of output: 212
Change import type to value import for PARAM_KEY.
PARAM_KEY is exported as a const value in constants.ts but imported as type-only in types.ts. Line 53 uses it as a computed property key [PARAM_KEY], which requires the symbol in the value space. A type-only import will fail here.
🔧 Proposed fix
-import type { PARAM_KEY } from './constants.ts';
+import { PARAM_KEY } from './constants.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.
Suggested change
| import type { PARAM_KEY } from './constants.ts'; | |
| import { PARAM_KEY } from './constants.ts'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/core/src/backgrounds/types.ts` at line 1, The import of PARAM_KEY is
currently type-only but it's used at runtime as a computed property key (e.g.
[PARAM_KEY]) so change the import from a type-only import to a normal value
import; replace "import type { PARAM_KEY }" with a value import for PARAM_KEY in
types.ts so the symbol is available at runtime where it's referenced (see usages
of PARAM_KEY around the computed property access).
Comment on lines
+5
to
+7
| import { executeCommand } from '../utils/command.ts'; | ||
| import { JsPackageManager } from './JsPackageManager.ts'; | ||
| import { Yarn2Proxy } from './Yarn2Proxy.ts'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="code/core/src/common/js-package-manager/Yarn2Proxy.test.ts"
python - <<'PY'
import re
from pathlib import Path
p = Path("code/core/src/common/js-package-manager/Yarn2Proxy.test.ts")
t = p.read_text()
imports = re.findall(r"import\s+\{\s*executeCommand\s*\}\s+from\s+'([^']+)'", t)
mocks = re.findall(r"vi\.mock\('([^']+)'", t)
print("executeCommand import specifier(s):", imports)
print("vi.mock specifier(s):", mocks)
print("MATCH" if imports and imports[0] in mocks else "MISMATCH")
PYRepository: storybookjs/storybook
Length of output: 210
🏁 Script executed:
# Check for Vitest configuration files that might affect module resolution
find . -type f \( -name "vitest.config.*" -o -name "vite.config.*" \) | head -20Repository: storybookjs/storybook
Length of output: 961
🏁 Script executed:
# Read the full test file to understand context and mock placement
cat -n code/core/src/common/js-package-manager/Yarn2Proxy.test.ts | head -60Repository: storybookjs/storybook
Length of output: 2283
🏁 Script executed:
# Read the vitest config for code/core to check for any extension resolution/alias settings
cat -n code/core/vitest.config.tsRepository: storybookjs/storybook
Length of output: 467
🏁 Script executed:
# Check the actual utils/command file to understand the module structure
cat -n code/core/src/common/utils/command.ts | head -20Repository: storybookjs/storybook
Length of output: 661
🏁 Script executed:
# Check the shared vitest config
cat -n code/vitest.shared.tsRepository: storybookjs/storybook
Length of output: 608
🏁 Script executed:
# Verify if the command.ts file actually exists at the expected path
ls -la code/core/src/common/utils/command.ts 2>&1 || echo "File not found"Repository: storybookjs/storybook
Length of output: 151
🏁 Script executed:
# Check if there's any extension resolution or alias configuration in vite-related configs
grep -r "extensions\|alias" code/vite.config.ts code/vitest.shared.ts 2>/dev/null || echo "No alias/extension config found in shared vitest config"Repository: storybookjs/storybook
Length of output: 120
🏁 Script executed:
# Verify the exact mock behavior by checking if there are similar patterns in other test files
rg "vi\.mock\(['\"]\.\./" code/core/src/common/js-package-manager/ -A 1 | head -30Repository: storybookjs/storybook
Length of output: 725
Align the mocked module specifier with the new .ts import.
Line 5 imports ../utils/command.ts, but the mock at Line 21 still targets ../utils/command without the extension. Per the coding guidelines requiring explicit file extensions for imports, the mock specifier must match for consistency and to ensure spy mocking works reliably.
🔧 Proposed fix
-vi.mock('../utils/command', { spy: true });
+vi.mock('../utils/command.ts', { spy: true });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/core/src/common/js-package-manager/Yarn2Proxy.test.ts` around lines 5 -
7, The test imports executeCommand from '../utils/command.ts' but the module
mock still targets '../utils/command' so the mock doesn't match; update the mock
specifier in Yarn2Proxy.test.ts to use the explicit extension (change any
mock/spy import of '../utils/command' to '../utils/command.ts') so that the
mocked executeCommand used by the Yarn2Proxy tests resolves correctly.
Comment on lines
+8
to
+9
| import * as resolveUtils from '../shared/utils/module.ts'; | ||
| import { getPresets, loadPreset, resolveAddonName } from './presets.ts'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify import-vs-mock specifier consistency in this test file.
file="code/core/src/common/presets.test.ts"
echo "== imports =="
rg -n "from '../shared/utils/module(\\.ts)?'" "$file"
echo
echo "== vi.mock target =="
rg -n "vi\\.mock\\('../shared/utils/module(\\.ts)?'" "$file"Repository: storybookjs/storybook
Length of output: 207
🏁 Script executed:
grep -n "mockedResolveUtils\|mockRe\|resolveUtils" code/core/src/common/presets.test.ts | head -30Repository: storybookjs/storybook
Length of output: 1115
Mock target must include .ts extension to match the import.
The import statement uses ../shared/utils/module.ts (with extension), but the mock target at Line 26 uses ../shared/utils/module (without extension). Vitest's module matching is string-based, so this mismatch prevents the mock from intercepting the imported module. The test extensively uses mockedResolveUtils.importModule.mockImplementation() in 11+ test cases, which will fail silently if the module is not properly mocked.
🔧 Fix
-vi.mock('../shared/utils/module', () => ({
+vi.mock('../shared/utils/module.ts', () => ({
importModule: vi.fn(),
safeResolveModule: vi.fn(({ specifier }) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/core/src/common/presets.test.ts` around lines 8 - 9, The mock target
string used in the vitest.mock call doesn't include the `.ts` extension so it
doesn't match the actual import and the mock of resolveUtils is not applied;
update the module string in the mock to include the `.ts` extension to match the
import (so tests that call
mockedResolveUtils.importModule.mockImplementation(...) will be intercepted),
then run the tests to confirm the mocked implementations are used.
Member
|
@copilot, can you bring this PR up to date with its base branch and resolve merge conflicts, please? |
…opilot/add-device-addons-property Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com>
Contributor
Author
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.test.ts`:
- Around line 19-21: The vitest mocks must include the { spy: true } option, use
explicit .ts extensions, and use vi.mocked() when accessing mocked functions:
change vi.mock('../helpers/mainConfigFile', () => ({ updateMainConfig:
mocks.updateMainConfig })) to vi.mock('../helpers/mainConfigFile.ts', () => ({
updateMainConfig: mocks.updateMainConfig }), { spy: true }), and replace any
direct uses of updateMainConfig in tests with vi.mocked(updateMainConfig) before
calling/inspecting it (e.g., wrap accesses in
vi.mocked(updateMainConfig).mockImplementation / .mock.calls etc.) so the mock
follows spy-mocking rules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 584ecdc5-61be-4b03-99de-a3e7aeea48c1
📒 Files selected for processing (6)
MIGRATION.mdcode/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.test.tscode/lib/create-storybook/src/generators/REACT_NATIVE/generateEntrypoint.test.tscode/lib/create-storybook/src/generators/REACT_NATIVE/generateScripts.test.tscode/lib/create-storybook/src/generators/REACT_NATIVE/index.test.tscode/lib/create-storybook/src/generators/REACT_NATIVE/metroConfig.test.ts
✅ Files skipped from review due to trivial changes (3)
- code/lib/create-storybook/src/generators/REACT_NATIVE/generateScripts.test.ts
- code/lib/create-storybook/src/generators/REACT_NATIVE/metroConfig.test.ts
- MIGRATION.md
Comment on lines
+19
to
+21
| vi.mock('../helpers/mainConfigFile', () => ({ | ||
| updateMainConfig: mocks.updateMainConfig, | ||
| })); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read the relevant sections
cat -n code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.test.ts | head -100Repository: storybookjs/storybook
Length of output: 4023
🏁 Script executed:
# Check the spy-mocking rules file
cat .cursor/rules/spy-mocking.mdc 2>/dev/null || echo "File not found at .cursor/rules/spy-mocking.mdc"Repository: storybookjs/storybook
Length of output: 1585
🏁 Script executed:
# Check the .cursorrules file for references
cat .cursorrules 2>/dev/null || echo "File not found"Repository: storybookjs/storybook
Length of output: 1827
🏁 Script executed:
# Search for other test files with similar vi.mock patterns to understand the convention
rg "vi\.mock\(" --type typescript --max-count 20 -B 2 -A 2 code/lib/cli-storybook/srcRepository: storybookjs/storybook
Length of output: 99
🏁 Script executed:
# Search for other vi.mock patterns in test files to understand the convention
rg "vi\.mock\(" --type ts --max-count 30 -B 2 -A 3 code/lib/cli-storybook/src --glob "*.test.ts"Repository: storybookjs/storybook
Length of output: 36806
🏁 Script executed:
# Check what pattern is used in other cli-storybook test files
find code/lib/cli-storybook/src -name "*.test.ts" | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && head -40 {}'Repository: storybookjs/storybook
Length of output: 5469
Align Vitest mocks with repo spy-mocking rules and explicit file extensions.
Three issues require fixing to comply with the Vitest spy-mocking standards:
- Missing
{ spy: true }option invi.mock() - Missing
.tsextension in the mock import path - Direct access to mocked function without
vi.mocked()wrapper at lines 33-40 and 266-269
Suggested patch
import { beforeEach, describe, expect, it, vi } from 'vitest';
import type { JsPackageManager } from 'storybook/internal/common';
import type { StorybookConfigRaw } from 'storybook/internal/types';
+import { updateMainConfig } from '../helpers/mainConfigFile.ts';
import { rnOndeviceAddonsToDeviceAddons } from './rn-ondevice-addons-to-device-addons.ts';
// vi.hoisted ensures these are available when vi.mock factories run (before module imports)
const mocks = vi.hoisted(() => {
const configFile = {
removeEntryFromArray: vi.fn(),
appendValueToArray: vi.fn(),
};
- const updateMainConfig = vi.fn();
- return { configFile, updateMainConfig };
+ return { configFile };
});
// Mock the updateMainConfig helper so we can assert on the AST manipulations.
-vi.mock('../helpers/mainConfigFile', () => ({
- updateMainConfig: mocks.updateMainConfig,
-}));
+vi.mock('../helpers/mainConfigFile.ts', { spy: true });
beforeEach(() => {
vi.clearAllMocks();
// Restore the implementation that invokes the callback with our mock ConfigFile
- mocks.updateMainConfig.mockImplementation(
+ vi.mocked(updateMainConfig).mockImplementation(
async (
_opts: { mainConfigPath: string; dryRun: boolean },
callback: (cfg: unknown) => Promise<void>
) => {
await callback(mocks.configFile);
}
);
});
- expect(mocks.updateMainConfig).toHaveBeenCalledWith(
+ expect(vi.mocked(updateMainConfig)).toHaveBeenCalledWith(
{ mainConfigPath: '.rnstorybook/main.ts', dryRun: true },
expect.any(Function)
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.test.ts`
around lines 19 - 21, The vitest mocks must include the { spy: true } option,
use explicit .ts extensions, and use vi.mocked() when accessing mocked
functions: change vi.mock('../helpers/mainConfigFile', () => ({
updateMainConfig: mocks.updateMainConfig })) to
vi.mock('../helpers/mainConfigFile.ts', () => ({ updateMainConfig:
mocks.updateMainConfig }), { spy: true }), and replace any direct uses of
updateMainConfig in tests with vi.mocked(updateMainConfig) before
calling/inspecting it (e.g., wrap accesses in
vi.mocked(updateMainConfig).mockImplementation / .mock.calls etc.) so the mock
follows spy-mocking rules.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/lib/cli-storybook/src/automigrate/fixes/index.ts`:
- Around line 37-38: The RN migration ordering in the allFixes list is wrong:
move rnstorybookConfig to appear immediately before
rnOndeviceAddonsToDeviceAddons so rnOndeviceAddonsToDeviceAddons runs right
after rnstorybookConfig; update the array in index.ts by swapping the two
entries (rnstorybookConfig and rnOndeviceAddonsToDeviceAddons) within the
allFixes export to preserve the intended execution order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c191b785-9beb-43bb-8dd8-710f04160cd3
📒 Files selected for processing (1)
code/lib/cli-storybook/src/automigrate/fixes/index.ts
6 tasks
This was referenced Apr 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes storybookjs/react-native#873
What I did
On-device addons listed in
addonscauseextract(story indexing / manifest generation) to fail in@storybook/react-nativeprojects because Storybook Core attempts to evaluate them as presets in a Node.js context. The on-device addons contain React Native code that cannot be loaded server-side, resulting in aCriticalPresetLoadError.The fix moves on-device addons to a separate
deviceAddonsfield that is only consumed at runtime (byscripts/generate.js), not by Core.Changes
rn-ondevice-addons-to-device-addons) — detects@storybook/react-nativeprojects and moves any addon whose name contains"ondevice"fromaddons→deviceAddons; preserves both string and object ({ name, options }) formsallFixesimmediately afterrnstorybookConfigReact Native: on-device addons moved to deviceAddonssection (with TOC entry and before/after example) in the 9.x → 10.0.0 chapterNote:
deviceAddonsis intentionally not added to the sharedStorybookConfigRawcore type since it is React Native-specific. The automigration usesConfigFile's string-path APIs to write the field without requiring a type declaration.Before / After:
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
.rnstorybook/main.ts > addons, confirm extract failsdeviceAddonsDocumentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
Documentation
Tests