-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Release: Patch 9.1.11 (retry) #32738
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Telemetry: Add metadata for react routers (cherry picked from commit 391bbc6)
…bpath-deployed-storybooks Mocking: Fix `sb.mock` usage in Storybook's deployed in subpaths (cherry picked from commit 25af7cd)
…_builder React Native Web: Fix REACT_NATIVE_AND_RNW should detect vite builder (cherry picked from commit a78bf07)
Release tooling: Use npm Trusted Publishing for canaries (cherry picked from commit 190788e)
Release tooling: Use npm Trusted Publishing (cherry picked from commit b0b5902)
…background-automigration Automigration: Improve the viewport/backgrounds automigration (cherry picked from commit a47e297)
…ification NextJS: Enhance PostCSS configuration handling (cherry picked from commit e9a2317)
…orybook into version-patch-from-9.1.10
(cherry picked from commit ec66cb6)
…next-config-in-Next-v16 Next.js: Remove next/config usage in Next.js >=v16 projects (cherry picked from commit b81780a)
📝 WalkthroughWalkthroughIntroduces a custom GitHub Actions composite action for Node.js setup and dependency installation, consolidates canary release automation into the publish workflow, updates Next.js frameworks with PostCSS auto-configuration and Next.js 16+ version-aware logic, extends automigration for viewport/backgrounds parameters, adds React router telemetry entries, and upgrades Yarn to 4.10.3 project-wide. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/CI
participant PS as PostCSS Search
participant PL as PostCSS Load
participant PN as Normalize
participant FS as File System
participant Retry as Retry Loop
User->>PS: searchPath → postCssFindConfig
PS-->>PN: config path or null
alt Config Found
PN->>PL: Load config as-is
alt Load Success
PL-->>User: return true
else Load Fails (Invalid Plugin)
PN->>FS: Read config file
FS-->>PN: file content
PN->>PN: Transform plugins array to object
PN->>FS: Write modified config
FS-->>Retry: file written
Retry->>PL: Retry loading normalized config
alt Retry Success
PL-->>User: return true
else Retry Fails
PN->>FS: Revert file changes
FS-->>User: throw IncompatiblePostCssConfigError
end
end
else No Config / No Error
PN-->>User: return true
else Non-Recoverable Error
PN-->>User: return false
end
sequenceDiagram
participant Workflow as publish.yml
participant Dispatch as workflow_dispatch/<br/>pull_request/<br/>push
participant Concurrency as Concurrency Handler
participant Setup as setup-node-and-install
participant Release as Release Steps
participant Publish as npm publish
Dispatch->>Workflow: trigger event
Workflow->>Concurrency: determine concurrency group
alt workflow_dispatch or pull_request
Concurrency->>Concurrency: group by PR number
Concurrency->>Concurrency: cancel in-progress
else push event
Concurrency->>Concurrency: group by branch name
end
Workflow->>Setup: call composite action
Setup-->>Workflow: Node.js + deps ready
alt push to latest-release/next-release<br/>(not [skip ci])
Workflow->>Release: publish-normal job
Release->>Release: version bump + changelog
Release->>Publish: npm publish with tag
Publish-->>Release: published
Release->>Release: merge branches + push
else Other triggers
Workflow->>Release: skip release flow
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The PR exhibits high heterogeneity: workflow consolidation and removal (canary-release-pr.yml), new composite action logic, multiple framework updates with version-aware conditionals and file I/O side effects, extended automigration with AST transformations, telemetry additions, and pervasive Yarn version updates. While many changes are routine (Yarn bumps, documentation), the logic density in PostCSS auto-configuration, Next.js version branching, and automigration extensions requires separate reasoning per area. Possibly related PRs
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 019e1a1
☁️ Nx Cloud last updated this comment at |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 50 | 50 | 0 |
| Self size | 35.91 MB | 35.93 MB | 🚨 +16 KB 🚨 |
| Dependency size | 16.65 MB | 16.65 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
sb
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 51 | 51 | 0 |
| Self size | 1 KB | 1 KB | 0 B |
| Dependency size | 52.56 MB | 52.58 MB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 220 | 220 | 0 |
| Self size | 598 KB | 598 KB | 0 B |
| Dependency size | 97.47 MB | 97.48 MB | 🚨 +18 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 189 | 189 | 0 |
| Self size | 31 KB | 31 KB | 0 B |
| Dependency size | 81.53 MB | 81.55 MB | 🚨 +18 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts (1)
140-166: Preview migration misses viewport defaultOrientation and disable handlingIn preview config, you set initialGlobals.viewport.value and hardcode isRotated: false, and you don’t handle parameters.viewport.disable → disabled. This diverges from the story migration (where defaultOrientation drives isRotated and disable is renamed). Apply the same logic for consistency.
if (needsViewportMigration) { // Get the viewport parameter object - const viewports = getFieldNode(['parameters', 'viewport', 'viewports']) as ObjectExpression; + const viewports = getFieldNode(['parameters', 'viewport', 'viewports']) as Expression; if (viewportsOptions?.viewports) { // Remove the old viewports property previewConfig.removeField(['parameters', 'viewport', 'viewports']); addProperty( getFieldNode(['parameters', 'viewport']) as ObjectExpression, 'options', viewports ); } // If defaultViewport exists, create initialGlobals.viewport if (viewportsOptions?.defaultViewport) { - // Remove the old defaultViewport property - const viewportNode = getFieldNode(['parameters', 'viewport']); - removeProperty(viewportNode as ObjectExpression, 'defaultViewport'); - - previewConfig.setFieldValue( - ['initialGlobals', 'viewport', 'value'], - viewportsOptions.defaultViewport - ); - previewConfig.setFieldValue(['initialGlobals', 'viewport', 'isRotated'], false); + const viewportNode = getFieldNode(['parameters', 'viewport']) as ObjectExpression; + // Compute isRotated from defaultOrientation when present + const defaultOrientation = getFieldNode([ + 'parameters', + 'viewport', + 'defaultOrientation', + ]); + const isRotated = + t.isStringLiteral(defaultOrientation) && defaultOrientation.value === 'portrait'; + + removeProperty(viewportNode, 'defaultViewport'); + removeProperty(viewportNode, 'defaultOrientation'); + + previewConfig.setFieldValue(['initialGlobals', 'viewport', 'value'], viewportsOptions.defaultViewport); + previewConfig.setFieldValue(['initialGlobals', 'viewport', 'isRotated'], isRotated); + } + + // If disable exists, rename to disabled + const previewViewport = getFieldNode(['parameters', 'viewport']) as ObjectExpression | undefined; + if (previewViewport) { + const disableViewport = getFieldNode(['parameters', 'viewport', 'disable']); + if (t.isBooleanLiteral(disableViewport)) { + removeProperty(previewViewport, 'disable'); + addProperty(previewViewport, 'disabled', disableViewport); + } } }[Based on learnings]
code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts (1)
17-26: Incorrect vi.mock usage for 'storybook/internal/babel' (will not mock)First argument to vi.mock must be the module id string, not a dynamic import. Use Vitest’s factory signature and optionally spy: true.
-vi.mock(import('storybook/internal/babel'), async (actualImport) => { - const actual = await actualImport(); +vi.mock('storybook/internal/babel', async (importOriginal) => { + const actual = await importOriginal<any>(); return { ...actual, recast: { ...actual.recast, print: (ast, options) => actual.recast.print(ast, { ...options, quote: 'single' }), }, }; -}); +}, { spy: true });As per coding guidelines
🧹 Nitpick comments (16)
code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts (4)
142-151: Over‑narrow type cast for viewports can break dynamic casesThe viewports node can be an Identifier/MemberExpression (e.g., INITIAL_VIEWPORTS). Casting to ObjectExpression is unsafe. Type it as Expression end-to-end.
-const viewports = getFieldNode(['parameters', 'viewport', 'viewports']) as ObjectExpression; +const viewports = getFieldNode(['parameters', 'viewport', 'viewports']) as Expression;
375-386: Backgrounds default key derivation should tolerate non-array “values”getKeyFromName is called with backgroundsOptions.values cast to ArrayExpression. If preview uses a reference (Identifier) rather than a literal array, this cast will be wrong. Add a fallback when values isn’t an ArrayExpression.
-const backgroundKey = getKeyFromName( - backgroundsOptions?.values as ArrayExpression, - defaultBackground.value -); +let backgroundKey: string; +const valuesExpr = backgroundsOptions?.values; +if (valuesExpr && t.isArrayExpression(valuesExpr)) { + backgroundKey = getKeyFromName(valuesExpr, defaultBackground.value); +} else { + // Fallback: normalize name to key similar to transformValuesToOptions + backgroundKey = defaultBackground.value.toLowerCase().replace(/\s+/g, '_'); +}
288-347: Viewport: Identifier defaultViewport values are ignoredOnly StringLiteral and MemberExpression are supported. If users set defaultViewport via a variable Identifier (const dv = 'mobile'), it won’t migrate. Consider accepting Identifier that resolves to a string literal at runtime.
-} else if (t.isMemberExpression(defaultViewport)) { +} else if (t.isMemberExpression(defaultViewport) || t.isIdentifier(defaultViewport)) { // Preserve the member/identifier as-is viewportValue = defaultViewport; }
404-466: Merging globals: do not mutate and re-use node instances across storiesYou push newGlobal nodes directly into existingGlobals, potentially reusing the same AST node between stories if the transformer reuses references. Clone nodes before insert to avoid cross-tree aliasing.
- existingGlobals.properties.push(newGlobal); + existingGlobals.properties.push(t.objectProperty( + t.identifier((newGlobal.key as t.Identifier).name), + t.isObjectExpression(newGlobal.value) + ? t.objectExpression([...newGlobal.value.properties]) + : newGlobal.value +));code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts (1)
15-16: Apply spy: true to fs mock for consistency with test guidelinesTests should use vi.mock(..., { spy: true }) and access mocks via vi.mocked(). You already use vi.mocked(), but add spy mode.
-vi.mock('node:fs/promises', async () => import('../../../../../__mocks__/fs/promises')); +vi.mock('node:fs/promises', async () => import('../../../../../__mocks__/fs/promises'), { + spy: true, +});As per coding guidelines
.github/actions/setup-node-and-install/action.yml (3)
18-21: Drop or pin the global npm update (Yarn is used).Unneeded here and can introduce variability. Prefer removing, or pin to a specific major if required elsewhere.
- - name: Update npm to latest - shell: bash - run: npm install -g npm@latest
13-17: Ensure Yarn 4 via Corepack.Add Corepack enable to guarantee the repo’s Yarn version is used across runners.
- name: Setup Node.js uses: actions/setup-node@v4 with: node-version-file: '.nvmrc' + - name: Enable Corepack (Yarn) + shell: bash + run: corepack enableAlso applies to: 33-37
22-32: Make caching OS‑agnostic and simpler by using setup‑node’s Yarn cache.actions/cache with a "~" path is unreliable on Windows. Let setup‑node manage Yarn caching with lockfile paths instead, and remove this step.
- - name: Cache dependencies - uses: actions/cache@v4 - with: - path: | - ~/.yarn/berry/cache - key: yarn-v1-${{ hashFiles('scripts/yarn.lock') }}-${{ hashFiles('code/yarn.lock') }} - restore-keys: | - yarn-v1-${{ hashFiles('scripts/yarn.lock') }}-${{ hashFiles('code/yarn.lock') }} - yarn-v1-${{ hashFiles('scripts/yarn.lock') }} - yarn-v1And extend setup‑node to cache Yarn based on both lockfiles:
- name: Setup Node.js uses: actions/setup-node@v4 with: node-version-file: '.nvmrc' + cache: 'yarn' + cache-dependency-path: | + scripts/yarn.lock + code/yarn.lockCONTRIBUTING/RELEASING.md (1)
453-454: Fix markdownlint MD034 (bare URL).Wrap the URL in angle brackets or convert to a markdown link to satisfy MD034.
-1. Open the workflow UI at https://github.com/storybookjs/storybook/actions/workflows/publish.yml +1. Open the workflow UI at <https://github.com/storybookjs/storybook/actions/workflows/publish.yml>Based on static analysis hints
.github/workflows/publish.yml (1)
186-199: Add [skip ci] to version JSON sync commit.Avoids unnecessary CI when syncing docs versions on prereleases.
- git commit -m "Update $VERSION_FILE for v$CURRENT_VERSION" + git commit -m "Update $VERSION_FILE for v$CURRENT_VERSION [skip ci]"code/core/src/telemetry/storybook-metadata.ts (1)
36-39: Naming consistency for metaFrameworks values.Existing values are capitalized tokens (e.g., 'Next', 'CRA', 'Gatsby'); new ones are lower‑case ('tanstack-react', 'react-router', 'remix'). Align casing/format to keep telemetry consistent, or confirm downstream expects these exact tokens.
code/frameworks/nextjs-vite/src/utils.ts (1)
3-3: Resolve next/package.json relative to the user project (avoid misresolution).Using bare require('next/package.json') can resolve to the wrong copy in monorepos/hoisted installs. Prefer resolving from the project root (parity with the webpack flavor).
Apply:
+import { getProjectRoot } from 'storybook/internal/common'; export const getNextjsVersion = (): string => require('next/package.json').version;And change to:
-export const getNextjsVersion = (): string => require('next/package.json').version; +export const getNextjsVersion = (): string => { + const pkgPath = require.resolve('next/package.json', { paths: [getProjectRoot()] }); + return require(pkgPath).version; +};code/frameworks/nextjs/src/config/webpack.ts (2)
64-75: Stringify DefinePlugin values for consistency.process.env.__NEXT_RUNTIME_CONFIG is stringified; do the same for __NEXT_NEW_LINK_BEHAVIOR to avoid injecting bare undefined/booleans as source tokens.
Apply:
- const newNextLinkBehavior = (nextConfig.experimental as any)?.newNextLinkBehavior; - definePluginConfig['process.env.__NEXT_NEW_LINK_BEHAVIOR'] = newNextLinkBehavior; + const newNextLinkBehavior = (nextConfig.experimental as any)?.newNextLinkBehavior; + definePluginConfig['process.env.__NEXT_NEW_LINK_BEHAVIOR'] = JSON.stringify(newNextLinkBehavior);
4-7: Compute version at call sites to avoid static capture (optional).isNext16orNewer is evaluated at module load. If tests change Next version or resolution context, recomputing where needed improves robustness. Low priority.
code/frameworks/nextjs-vite/src/find-postcss-config.ts (1)
50-56: Outdated JSDoc type annotation style.The JSDoc uses older capitalized type syntax (
@param {String},@param {Object}) instead of TypeScript's lowercase primitives (string,object). While this doesn't affect runtime, consider updating for consistency with modern TypeScript conventions./** * Find PostCSS config file path (without loading the config) * - * @param {String} path Config Path - * @param {Object} options Config Options + * @param {string} path Config Path + * @param {object} options Config Options * @returns {Promise<string | null>} Config file path or null if not found */docs/versions/next.json (1)
1-1: Polish release notes wording
- Change “Fix
sb.mockusage in Storybook's deployed in subpaths” → “Fixsb.mockusage for Storybooks deployed under subpaths”- Change “Nextjs” → “Next.js” (in title and description)
- Change “Fix REACT_NATIVE_AND_RNW should detect vite builder” → “Ensure REACT_NATIVE_AND_RNW detects the Vite builder”
docs/versions JSON files are intentionally one-line (matching latest.json); no Prettier run required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
.yarn/releases/yarn-4.10.3.cjsis excluded by!**/.yarn/**.yarn/releases/yarn-4.9.1.cjsis excluded by!**/.yarn/**code/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (40)
.github/actions/setup-node-and-install/action.yml(1 hunks).github/workflows/canary-release-pr.yml(0 hunks).github/workflows/generate-sandboxes.yml(2 hunks).github/workflows/prepare-non-patch-release.yml(3 hunks).github/workflows/prepare-patch-release.yml(3 hunks).github/workflows/publish.yml(7 hunks).github/workflows/tests-unit.yml(1 hunks).yarnrc.yml(1 hunks)CHANGELOG.md(1 hunks)CONTRIBUTING/RELEASING.md(2 hunks)code/.yarnrc.yml(1 hunks)code/core/src/cli/detect.ts(1 hunks)code/core/src/core-server/presets/vitePlugins/vite-inject-mocker/plugin.ts(1 hunks)code/core/src/telemetry/storybook-metadata.ts(1 hunks)code/frameworks/nextjs-vite/package.json(2 hunks)code/frameworks/nextjs-vite/src/find-postcss-config.ts(1 hunks)code/frameworks/nextjs-vite/src/preset.ts(3 hunks)code/frameworks/nextjs-vite/src/preview.tsx(1 hunks)code/frameworks/nextjs-vite/src/utils.ts(1 hunks)code/frameworks/nextjs/package.json(2 hunks)code/frameworks/nextjs/src/config/webpack.ts(3 hunks)code/frameworks/nextjs/src/preset.ts(2 hunks)code/frameworks/nextjs/src/preview.tsx(0 hunks)code/frameworks/nextjs/src/utils.ts(2 hunks)code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts(17 hunks)code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts(5 hunks)code/package.json(1 hunks)docs/versions/latest.json(1 hunks)docs/versions/next.json(1 hunks)package.json(1 hunks)scripts/.yarnrc.yml(1 hunks)scripts/package.json(1 hunks)test-storybooks/ember-cli/.yarnrc.yml(1 hunks)test-storybooks/external-docs/.yarnrc.yml(1 hunks)test-storybooks/portable-stories-kitchen-sink/nextjs/.yarnrc.yml(1 hunks)test-storybooks/portable-stories-kitchen-sink/react/.yarnrc.yml(1 hunks)test-storybooks/portable-stories-kitchen-sink/svelte/.yarnrc.yml(1 hunks)test-storybooks/portable-stories-kitchen-sink/vue3/.yarnrc.yml(1 hunks)test-storybooks/server-kitchen-sink/.yarnrc.yml(1 hunks)test-storybooks/standalone-preview/.yarnrc.yml(1 hunks)
💤 Files with no reviewable changes (2)
- code/frameworks/nextjs/src/preview.tsx
- .github/workflows/canary-release-pr.yml
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/frameworks/nextjs-vite/src/utils.tsscripts/package.jsoncode/frameworks/nextjs-vite/src/preview.tsxcode/package.jsoncode/frameworks/nextjs-vite/package.jsoncode/frameworks/nextjs/src/config/webpack.tscode/frameworks/nextjs/src/utils.tscode/frameworks/nextjs-vite/src/preset.tscode/core/src/cli/detect.tscode/core/src/telemetry/storybook-metadata.tscode/core/src/core-server/presets/vitePlugins/vite-inject-mocker/plugin.tscode/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.tspackage.jsoncode/frameworks/nextjs/package.jsondocs/versions/latest.jsoncode/frameworks/nextjs-vite/src/find-postcss-config.tsdocs/versions/next.jsoncode/frameworks/nextjs/src/preset.tscode/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/frameworks/nextjs-vite/src/utils.tscode/frameworks/nextjs-vite/src/preview.tsxcode/frameworks/nextjs/src/config/webpack.tscode/frameworks/nextjs/src/utils.tscode/frameworks/nextjs-vite/src/preset.tscode/core/src/cli/detect.tscode/core/src/telemetry/storybook-metadata.tscode/core/src/core-server/presets/vitePlugins/vite-inject-mocker/plugin.tscode/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.tscode/frameworks/nextjs-vite/src/find-postcss-config.tscode/frameworks/nextjs/src/preset.tscode/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/frameworks/nextjs-vite/src/utils.tscode/frameworks/nextjs-vite/src/preview.tsxcode/frameworks/nextjs/src/config/webpack.tscode/frameworks/nextjs/src/utils.tscode/frameworks/nextjs-vite/src/preset.tscode/core/src/cli/detect.tscode/core/src/telemetry/storybook-metadata.tscode/core/src/core-server/presets/vitePlugins/vite-inject-mocker/plugin.tscode/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.tscode/frameworks/nextjs-vite/src/find-postcss-config.tscode/frameworks/nextjs/src/preset.tscode/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/frameworks/nextjs-vite/src/utils.tscode/frameworks/nextjs-vite/src/preview.tsxcode/frameworks/nextjs/src/config/webpack.tscode/frameworks/nextjs/src/utils.tscode/frameworks/nextjs-vite/src/preset.tscode/core/src/cli/detect.tscode/core/src/telemetry/storybook-metadata.tscode/core/src/core-server/presets/vitePlugins/vite-inject-mocker/plugin.tscode/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.tscode/frameworks/nextjs-vite/src/find-postcss-config.tscode/frameworks/nextjs/src/preset.tscode/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Update documentation under
docs/for significant changes, including migration guides for breaking changes
Files:
docs/versions/latest.jsondocs/versions/next.json
code/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Files:
code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together
Files:
code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts
**/*.@(test|spec).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.@(test|spec).{ts,tsx,js,jsx}: Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Mock external dependencies in tests usingvi.mock()(e.g., filesystem, loggers)
Files:
code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts
🧠 Learnings (7)
📚 Learning: 2025-10-02T09:22:13.215Z
Learnt from: JReinhold
PR: storybookjs/storybook#32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
Applied to files:
test-storybooks/ember-cli/.yarnrc.ymlscripts/package.jsoncode/package.jsontest-storybooks/portable-stories-kitchen-sink/svelte/.yarnrc.ymltest-storybooks/portable-stories-kitchen-sink/vue3/.yarnrc.ymltest-storybooks/standalone-preview/.yarnrc.ymltest-storybooks/external-docs/.yarnrc.ymltest-storybooks/portable-stories-kitchen-sink/react/.yarnrc.ymlscripts/.yarnrc.ymltest-storybooks/server-kitchen-sink/.yarnrc.yml.yarnrc.ymlcode/.yarnrc.ymlpackage.jsontest-storybooks/portable-stories-kitchen-sink/nextjs/.yarnrc.yml
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.@(test|spec).{ts,tsx,js,jsx} : Mock external dependencies in tests using `vi.mock()` (e.g., filesystem, loggers)
Applied to files:
code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in beforeEach blocks
Applied to files:
code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mocked() to access and implement mock behaviors
Applied to files:
code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Applied to files:
code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors
Applied to files:
code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses
Applied to files:
code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts
🧬 Code graph analysis (7)
code/frameworks/nextjs-vite/src/utils.ts (1)
code/core/src/manager-api/version.ts (1)
version(1-1)
code/frameworks/nextjs/src/config/webpack.ts (2)
code/frameworks/nextjs-vite/src/utils.ts (1)
isNextVersionGte(5-9)code/frameworks/nextjs/src/utils.ts (2)
isNextVersionGte(23-27)addScopedAlias(73-77)
code/frameworks/nextjs-vite/src/preset.ts (2)
code/frameworks/nextjs-vite/src/utils.ts (1)
isNextVersionGte(5-9)code/frameworks/nextjs-vite/src/find-postcss-config.ts (1)
normalizePostCssConfig(84-136)
code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts (1)
code/lib/cli-storybook/src/automigrate/helpers/ast-utils.ts (4)
getObjectProperty(7-23)removeProperty(26-41)addProperty(44-50)transformValuesToOptions(78-103)
code/frameworks/nextjs-vite/src/find-postcss-config.ts (2)
code/core/src/common/utils/paths.ts (1)
getProjectRoot(9-58)code/core/src/server-errors.ts (1)
IncompatiblePostCssConfigError(540-568)
code/frameworks/nextjs/src/preset.ts (1)
code/frameworks/nextjs/src/utils.ts (1)
isNextVersionGte(23-27)
code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts (2)
code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts (1)
transformStoryFile(265-467)code/core/src/csf-tools/CsfFile.ts (1)
printCsf(903-905)
🪛 markdownlint-cli2 (0.18.1)
CONTRIBUTING/RELEASING.md
453-453: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: daily
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (46)
code/core/src/core-server/presets/vitePlugins/vite-inject-mocker/plugin.ts (1)
11-11: Manual verification needed: subpath mocker script loading
No automated tests cover subpath deployments forvite-inject-mocker-entry.js. Please manually confirm that the injected script./vite-inject-mocker-entry.jsloads correctly when Storybook is served from a subpath.code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts (3)
44-61: Transform function pipeline LGTMSwitching tests to transformStoryFile + printCsf mirrors the new public API and keeps snapshots stable via single-quote recast override.
232-267: Preview migration snapshots cover viewport options + initialGlobalsSnapshots reflect options promotion and initialGlobals.viewport with isRotated false. Once preview defaultOrientation handling is added, update related snapshots to assert isRotated accordingly.
834-868: New viewport story cases validated (defaultOrientation/disable/member expression)Good coverage for orientation, disable rename, and member-expression values. These rely on the story-side logic; keep in sync with preview-side changes.
.github/workflows/generate-sandboxes.yml (2)
123-127: Good: consistent pinning across jobs.Same note as above; looks solid.
71-75: All Ilshidur/action-discord usages in workflows are pinned to SHA d2594079a10f1d6739ee50a2471f0ca57418b554.CONTRIBUTING/RELEASING.md (2)
463-464: CLI example looks correct.Using publish.yml with the pr field aligns with the new workflow.
445-446: Clearer security callout.The warning reads well; no changes needed.
.github/workflows/prepare-non-patch-release.yml (3)
40-41: Verify environment name casing.GitHub environments are case‑sensitive. Ensure “Release” exists (and replaces any prior “release”) in repo settings.
54-56: Good consolidation to composite setup.Assuming the composite action enables Corepack and uses setup‑node Yarn caching; if not, please incorporate those to avoid Yarn v1 fallback and OS‑specific cache misses.
145-146: Good: Discord action pinned to a commit.Security hardening acknowledged.
.github/workflows/prepare-patch-release.yml (3)
22-23: Verify environment name casing.Confirm “Release” environment exists and is intended.
33-35: Good consolidation to composite setup.Keeps workflows DRY and consistent.
168-169: Good: Discord action pinned to a commit.Consistent with other workflows.
.github/workflows/tests-unit.yml (1)
22-26: LGTM: unified setup via composite action.Keeps unit tests consistent with other workflows. Ensure the composite action enables Corepack and uses setup‑node Yarn caching for Windows runners.
.github/workflows/publish.yml (4)
25-29: Good: OIDC permission enabled.id-token: write is required for npm Trusted Publishing.
30-36: Concurrency grouping looks sound.Prevents overlapping runs per PR/branch; cancel‑in‑progress for canaries is appropriate.
56-60: Good: consolidate setup via composite action.Keeps publish flow consistent. Ensure corepack/caching improvements are in the action.
212-218: Good: Discord action pinned to a commit.Consistent with the rest of the workflows.
test-storybooks/ember-cli/.yarnrc.yml (1)
3-3: LGTM!The Yarn version bump to 4.10.3 is consistent with the repository-wide upgrade and aligns with the project's use of Yarn v^4.
Based on learnings
scripts/.yarnrc.yml (1)
5-5: LGTM!The Yarn binary update to 4.10.3 is consistent with the repository-wide upgrade.
Based on learnings
test-storybooks/portable-stories-kitchen-sink/svelte/.yarnrc.yml (1)
7-7: LGTM!The Yarn version update to 4.10.3 is correct and consistent with the repository-wide upgrade.
Based on learnings
test-storybooks/external-docs/.yarnrc.yml (1)
3-3: LGTM!The Yarn version bump to 4.10.3 is consistent with the repository-wide upgrade.
Based on learnings
test-storybooks/portable-stories-kitchen-sink/vue3/.yarnrc.yml (1)
7-7: LGTM!The Yarn version update to 4.10.3 is correct and aligns with the repository-wide upgrade.
Based on learnings
code/core/src/cli/detect.ts (1)
136-138: LGTM!The addition of
ProjectType.REACT_NATIVE_AND_RNWcase correctly ensures these projects use the Vite builder, consistent with theREACT_NATIVE_WEBhandling.test-storybooks/portable-stories-kitchen-sink/react/.yarnrc.yml (1)
1-1: LGTM!The Yarn version update to 4.10.3 is correct and consistent with the repository-wide upgrade.
Based on learnings
code/.yarnrc.yml (1)
5-5: LGTM!The Yarn binary update to 4.10.3 is consistent with the repository-wide upgrade and aligns with the project's use of Yarn v^4.
Based on learnings
test-storybooks/portable-stories-kitchen-sink/nextjs/.yarnrc.yml (1)
3-3: LGTM! Yarn version upgrade aligned with repository-wide update.The upgrade from Yarn 4.9.1 to 4.10.3 is part of a consistent repository-wide update and follows the established pattern of using Yarn v^4.
Based on learnings
test-storybooks/standalone-preview/.yarnrc.yml (1)
3-3: LGTM! Yarn version upgrade consistent across test storybooks.This change aligns with the repository-wide Yarn 4.10.3 upgrade.
Based on learnings
code/package.json (2)
242-242: LGTM! Package manager upgrade aligned with repository-wide update.The Yarn version upgrade from 4.9.1 to 4.10.3 is consistent with the broader repository update.
Based on learnings
247-247: LGTM! Release version tracking metadata.The
deferredNextVersionfield appropriately tracks the upcoming patch release version (9.1.11) for this release preparation PR.test-storybooks/server-kitchen-sink/.yarnrc.yml (1)
3-3: LGTM! Yarn version upgrade consistent with repository standard.This update aligns with the repository-wide Yarn 4.10.3 upgrade.
Based on learnings
code/frameworks/nextjs-vite/package.json (1)
120-123: LGTM! New devDependencies support preview config functionality.The additions of
lilconfigandsemverare appropriate devDependencies for configuration loading and version checking, likely used by the new preview config module.code/frameworks/nextjs/package.json (2)
69-69: LGTM! Preview config export properly aligned with bundler configuration.The new export
"./config/preview"correctly points to the bundled output, and the bundler entry at line 208 ensures the source file is included in the build.
208-208: LGTM! Preview config source properly configured for bundling.The bundler entry
"./src/config/preview.ts"ensures the new preview configuration module is included in the build output, supporting the export added at line 69.code/frameworks/nextjs/src/utils.ts (1)
23-27: Utility looks good; canary-friendly comparison via coerce.Logic is sound and safely defaults to false when unparsable.
code/frameworks/nextjs-vite/src/utils.ts (1)
5-9: LGTM on version comparison helper.Coercion handles canary tags and returns a safe boolean.
code/frameworks/nextjs/src/config/webpack.ts (1)
25-28: Alias gating for Next <16 is correct.Only aliasing next/config for pre‑16 avoids overriding built‑in v16 behavior.
code/frameworks/nextjs/src/preset.ts (1)
54-64: Verified: dist/config/preview.mjs is correctly exported
src/config/preview.ts compiles to dist/config/preview.mjs and is listed under "./config/preview" in package.json.code/frameworks/nextjs-vite/src/preset.ts (3)
11-13: LGTM!The new imports for PostCSS configuration normalization and Next.js version checking are well-placed and align with the refactored logic below.
32-44: Version-aware preview annotations look correct.The logic appropriately adds conditional preview annotations based on the Next.js version. For Next.js <16, the additional
dist/config/preview.mjsis included, which will be removed once only v16+ is supported.
56-61: Error propagation verified
normalizePostCssConfig is awaited without a try/catch in preset.ts, so any thrown IncompatiblePostCssConfigError will bubble up and halt startup..yarnrc.yml (1)
11-11: LGTM!The Yarn version upgrade from 4.9.1 to 4.10.3 is consistent with the repository's policy of using Yarn v4.x and aligns with the corresponding updates in package.json. Based on learnings.
package.json (1)
22-22: LGTM!The
packageManagerfield update to[email protected]is consistent with the Yarn version upgrade in .yarnrc.yml and aligns with the repository's use of Yarn v4.x. Based on learnings.docs/versions/latest.json (1)
1-1: LGTM!The version bump to 9.1.11 and the release notes accurately reflect the bug fixes included in this patch release. Each item is properly attributed with PR references and contributors.
scripts/package.json (1)
192-192: Yarn bump looks good; confirm engines removal intent and CI alignment.Aligns with repo’s Yarn v4 usage. Please confirm the engines.node removal here (and repo‑wide) is intentional and matches CI/runtime expectations, given several deps/types target Node 18–22. Also ensure Prettier/ESLint ran on this file.
As per coding guidelines
| "import": "./dist/export-mocks/cache/index.mjs", | ||
| "require": "./dist/export-mocks/cache/index.js" | ||
| }, | ||
| "./config/preview": "./dist/config/preview.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify that the preview config source is bundled correctly.
The new export "./config/preview" points to "./dist/config/preview.js", but the provided bundler configuration (lines 145-156) does not show this file as an entry point. Ensure that:
- The source file (e.g.,
src/config/preview.tsorsrc/config/preview.js) exists - The bundler configuration includes this file in its entries
🏁 Script executed:
#!/bin/bash
# Description: Verify the preview config source exists and is configured for bundling
# Check if preview config source exists
fd -t f 'preview' code/frameworks/nextjs-vite/src/config/
# Check bundler entries in the full package.json
cat code/frameworks/nextjs-vite/package.json | jq '.bundler.entries[]' | grep -i previewLength of output: 69
Include preview config in bundler entries
The source src/config/preview.ts exists but bundler.entries only includes src/preview.tsx. Add "./src/config/preview.ts" to bundler.entries in code/frameworks/nextjs-vite/package.json so that dist/config/preview.js is generated.
🤖 Prompt for AI Agents
In code/frameworks/nextjs-vite/package.json around line 45, bundler.entries
currently only includes "./src/preview.tsx" but the source src/config/preview.ts
is missing; update the bundler.entries array to also include
"./src/config/preview.ts" so the bundler emits dist/config/preview.js during
build, ensuring the JSON entry list includes that path alongside the existing
entries.
| async function loader(filepath: string) { | ||
| return require(filepath); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential ESM compatibility issue with require().
The loader function uses require() to load config files. This could fail in ESM-only contexts or cause issues with native ESM config files (.mjs, .mts). Consider using dynamic import() instead for better ESM compatibility.
Apply this diff to use dynamic import:
-async function loader(filepath: string) {
- return require(filepath);
-}
+async function loader(filepath: string) {
+ return import(filepath);
+}📝 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.
| async function loader(filepath: string) { | |
| return require(filepath); | |
| } | |
| async function loader(filepath: string) { | |
| return import(filepath); | |
| } |
🤖 Prompt for AI Agents
In code/frameworks/nextjs-vite/src/find-postcss-config.ts around lines 11 to 13,
the loader uses require(filepath) which breaks in ESM-only contexts; replace it
with an asynchronous dynamic import by converting the filepath to a file:// URL
(using url.pathToFileURL) and await import(url), then return module.default ??
module so both default and named exports are handled; this keeps the function
async and compatible with .mjs/.mts ESM configs while still supporting CJS
exports.
| */ export const normalizePostCssConfig = async (searchPath: string): Promise<boolean> => { | ||
| const configPath = await postCssFindConfig(searchPath); | ||
| if (!configPath) { | ||
| return true; | ||
| } | ||
|
|
||
| let error: Error | undefined; | ||
|
|
||
| // First attempt: try loading config as-is | ||
| try { | ||
| await postCssLoadConfig({}, searchPath, { stopDir: getProjectRoot() }); | ||
| return true; // Success! | ||
| } catch (e: unknown) { | ||
| if (e instanceof Error) { | ||
| error = e; | ||
| } | ||
| } | ||
|
|
||
| if (!error) { | ||
| return true; | ||
| } | ||
|
|
||
| // No config found is not an error we need to handle | ||
| if (error.message.includes('No PostCSS Config found')) { | ||
| return true; | ||
| } | ||
|
|
||
| // NextJS uses an incompatible format for PostCSS plugins, we make an attempt to fix it | ||
| if (error.message.includes('Invalid PostCSS Plugin found')) { | ||
| // Second attempt: try with modified config | ||
| const originalContent = await readFile(configPath, 'utf8'); | ||
| try { | ||
| const modifiedContent = originalContent.replace( | ||
| 'plugins: ["@tailwindcss/postcss"]', | ||
| 'plugins: { "@tailwindcss/postcss": {} }' | ||
| ); | ||
|
|
||
| // Write the modified content | ||
| await writeFile(configPath, modifiedContent, 'utf8'); | ||
|
|
||
| // Retry loading the config | ||
| await postCssLoadConfig({}, searchPath, { stopDir: getProjectRoot() }); | ||
| return true; // Success with modified config! | ||
| } catch { | ||
| // We were unable to fix the config, so we change the file back to the original content | ||
| await writeFile(configPath, originalContent, 'utf8'); | ||
| // and throw an error | ||
| throw new IncompatiblePostCssConfigError({ error }); | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String replacement is too narrow and fragile.
The automatic fix at lines 116-119 only handles the exact string 'plugins: ["@tailwindcss/postcss"]'. This will fail for:
- Different plugin names
- Arrays with multiple plugins (e.g.,
["plugin1", "plugin2"]) - Different quote styles (single vs double quotes)
- Different whitespace or formatting
- Multi-line arrays
Consider either:
- Using a more robust AST-based transformation (e.g., via
jscodeshiftor similar) - Documenting the limitation clearly and providing a more helpful error message when the automatic fix fails
- Expanding the regex to handle more cases
Example of a more flexible regex-based approach:
- const modifiedContent = originalContent.replace(
- 'plugins: ["@tailwindcss/postcss"]',
- 'plugins: { "@tailwindcss/postcss": {} }'
- );
+ // Match: plugins: ["plugin-name"] or plugins: ['plugin-name']
+ const modifiedContent = originalContent.replace(
+ /plugins:\s*\[\s*["']([^"']+)["']\s*\]/,
+ 'plugins: { "$1": {} }'
+ );Note: Even this regex is limited and won't handle multi-plugin arrays. For production robustness, consider AST-based transformation.
🤖 Prompt for AI Agents
In code/frameworks/nextjs-vite/src/find-postcss-config.ts around lines 84 to
136, the current string-replacement fix for PostCSS plugins is too narrow (only
replaces the exact string 'plugins: ["@tailwindcss/postcss"]') and will fail for
different plugin names, quote styles, spacing, multi-line arrays or multiple
plugins; replace this brittle approach with a robust AST-based transformation:
parse the config file (e.g., with @babel/parser or recast), locate the exported
object/assignment that contains the plugins property, convert array plugin
entries that are string literals into an object map (pluginName: {}) while
preserving other array entries and comments, serialize the modified AST back to
source and write it to disk (keep a backup of originalContent and restore on any
failure), then retry postCssLoadConfig; if AST transformation cannot be applied,
do not attempt a blind string change—restore the original file and throw
IncompatiblePostCssConfigError with a clear message explaining the limitation
and suggesting manual migration or the tool’s supported conversion rules.
| const originalContent = await readFile(configPath, 'utf8'); | ||
| try { | ||
| const modifiedContent = originalContent.replace( | ||
| 'plugins: ["@tailwindcss/postcss"]', | ||
| 'plugins: { "@tailwindcss/postcss": {} }' | ||
| ); | ||
|
|
||
| // Write the modified content | ||
| await writeFile(configPath, modifiedContent, 'utf8'); | ||
|
|
||
| // Retry loading the config | ||
| await postCssLoadConfig({}, searchPath, { stopDir: getProjectRoot() }); | ||
| return true; // Success with modified config! | ||
| } catch { | ||
| // We were unable to fix the config, so we change the file back to the original content | ||
| await writeFile(configPath, originalContent, 'utf8'); | ||
| // and throw an error | ||
| throw new IncompatiblePostCssConfigError({ error }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File modification lacks atomic write protection.
The function reads, modifies, and writes the PostCSS config file (lines 114-122) without atomic write protection. If the process crashes or is interrupted between the write (line 122) and the rollback (line 129), the file could be left in a modified state.
Consider using:
- Write to a temporary file first, then atomic rename
- Or add a file lock mechanism
- Or document this limitation and ensure users have version control
Example using atomic write pattern:
import { rename } from 'node:fs/promises';
// Inside the try block:
const tempPath = `${configPath}.tmp`;
await writeFile(tempPath, modifiedContent, 'utf8');
// Retry loading the config
await postCssLoadConfig({}, searchPath, { stopDir: getProjectRoot() });
// If successful, replace original
await rename(tempPath, configPath);
return true;In the catch block, clean up the temp file instead of restoring:
try {
await unlink(tempPath);
} catch {
// Ignore cleanup errors
}🤖 Prompt for AI Agents
In code/frameworks/nextjs-vite/src/find-postcss-config.ts, lines 114 to 132, the
function modifies the PostCSS config file without atomic write protection,
risking partial writes if interrupted. Fix this by writing the modified content
to a temporary file first, then atomically renaming it to replace the original.
In the catch block, remove any temporary files instead of restoring the original
content to ensure file consistency and avoid partial writes.
| } | ||
|
|
||
| return false; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unclear when return false is reached.
The function returns false at line 135, but it's unclear under what conditions this line would be reached. If an error doesn't match the expected patterns (lines 107 or 112), should the function throw instead of silently returning false?
Consider either:
- Removing the
return falseand throwing an error for unexpected cases - Adding explicit logic that determines when to return
false - Documenting what
falsemeans to the caller
}
- return false;
+ // If we reach here, we encountered an unexpected error
+ throw new IncompatiblePostCssConfigError({ error: error || new Error('Unknown PostCSS config error') });
};📝 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.
| } | |
| return false; | |
| }; | |
| } | |
| // If we reach here, we encountered an unexpected error | |
| throw new IncompatiblePostCssConfigError({ | |
| error: error || new Error('Unknown PostCSS config error'), | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In code/frameworks/nextjs-vite/src/find-postcss-config.ts around lines 133–136,
the function currently falls through to a silent "return false" which makes it
unclear when that path is valid; replace this ambiguous behavior by throwing a
descriptive error for any unexpected/fall-through case and only return false
explicitly in a well-documented, clearly-checked branch (or vice-versa: document
the meaning of false and add an explicit condition that produces it). Update the
function so unexpected error patterns result in a thrown Error with context (so
callers can handle or surface failures), add a short inline comment explaining
the semantic meaning of returning false, and add/adjust tests to cover both the
documented false condition and the new thrown-error path.
| import type { Addon_DecoratorFunction, LoaderFunction } from 'storybook/internal/types'; | ||
|
|
||
| import type { ReactRenderer, StoryFn } from '@storybook/react'; | ||
| import type { ReactRenderer } from '@storybook/react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify that removed imports are not required.
The AI summary indicates that StoryFn from @storybook/react and the local import ./config/preview were removed. Ensure that:
StoryFnwas not used elsewhere in this file- Removing the
./config/previewimport doesn't break any side-effects or setup required for the preview
🏁 Script executed:
#!/bin/bash
# Description: Verify that StoryFn and ./config/preview are not referenced elsewhere in this file
# Check if StoryFn is used in the file
rg -n 'StoryFn' code/frameworks/nextjs-vite/src/preview.tsx
# Check if config/preview module exists and what it exports
fd -t f 'preview' code/frameworks/nextjs-vite/src/config/
# Check if any other files import from nextjs-vite/config/preview
rg -n "from '@storybook/nextjs-vite/config/preview'" --type ts --type tsxLength of output: 77
🏁 Script executed:
#!/bin/bash
# Inspect preview.tsx for imports and usage
sed -n '1,50p' code/frameworks/nextjs-vite/src/preview.tsx
echo "---- config/preview.ts ----"
# Inspect config/preview.ts for side-effects or exports
sed -n '1,200p' code/frameworks/nextjs-vite/src/config/preview.tsLength of output: 2190
Restore the ./config/preview import in preview.tsx.
config/preview.ts invokes setConfig(process.env.__NEXT_RUNTIME_CONFIG) as a side-effect—without importing it, Next’s runtime config won’t be initialized in Storybook. Re-add import '../config/preview'; at the top of preview.tsx.
🤖 Prompt for AI Agents
In code/frameworks/nextjs-vite/src/preview.tsx around line 5, the side-effect
import for initializing Next runtime config was removed; re-add the import
'../config/preview'; at the top of the file so that config/preview.ts executes
setConfig(process.env.__NEXT_RUNTIME_CONFIG) during Storybook startup, ensuring
Next’s runtime config is initialized before other preview code runs.
| @@ -0,0 +1,9 @@ | |||
| import semver from 'semver'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Confirm semver is available in this package too.
This module imports semver; ensure it’s declared in the relevant package.json to avoid runtime resolution issues.
Run:
🏁 Script executed:
#!/bin/bash
rg -nP '"semver"\s*:\s*"' -C2 code/**/package.json package.jsonLength of output: 242
Add semver to dependencies in code/frameworks/nextjs-vite/package.json
This module imports semver but it’s not listed in package.json; add it to avoid runtime resolution errors.
🤖 Prompt for AI Agents
In code/frameworks/nextjs-vite/src/utils.ts around line 1, the file imports the
external module "semver" but that package is not declared in
code/frameworks/nextjs-vite/package.json; update that package.json by adding
"semver" to the dependencies (use an appropriate semver version or caret range
consistent with the repo's dependency policy, e.g., "^7.3.0" or the latest
stable), run yarn/npm install in that package to update lockfile, and commit the
updated package.json and lockfile so the module resolves at runtime.
| import semver from 'semver'; | ||
|
|
||
| export const getNextjsVersion = (): string => require('next/package.json').version; | ||
|
|
||
| export const isNextVersionGte = (version: string): boolean => { | ||
| const currentVersion = getNextjsVersion(); | ||
| const coercedVersion = semver.coerce(currentVersion); | ||
| return coercedVersion ? semver.gte(coercedVersion, version) : false; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Release sanity check: version bump validation.
Given this is a patch release PR, ensure all hardcoded Storybook version strings are updated to 9.1.11 (e.g., manager-api/version.ts, package manifests).
Run:
🏁 Script executed:
#!/bin/bash
# Find any remaining 9.1.10 references
rg -nP '\b9\.1\.10\b' -g '!**/CHANGELOG*.md' -S
# Inspect declared version fields
rg -nP '"version"\s*:\s*"' -g 'code/**/package.json' -nLength of output: 8632
Update all hardcoded Storybook version strings to 9.1.11
Replace every remaining '9.1.10' in code/core/src/manager-api/version.ts, code/core/src/common/versions.ts and all code/**/package.json files with 9.1.11.
🤖 Prompt for AI Agents
In code/frameworks/nextjs-vite/src/utils.ts lines 1-9 and across the repo,
replace all occurrences of the hardcoded Storybook version '9.1.10' with
'9.1.11' — specifically update code/core/src/manager-api/version.ts,
code/core/src/common/versions.ts and every code/**/package.json that contains
'9.1.10' to '9.1.11'; ensure you only change the version string (preserve
surrounding formatting and quotes), run a quick grep to find remaining
occurrences, and update package.json files' version entries without altering
other fields.
| import type { NextConfig } from 'next'; | ||
| import { PHASE_DEVELOPMENT_SERVER } from 'next/constants.js'; | ||
| import nextJsLoadConfigModule from 'next/dist/server/config.js'; | ||
| import semver from 'semver'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Ensure semver is declared for this package.
Confirm semver is a dependency of the workspace/package that builds this module.
Run to verify:
🏁 Script executed:
#!/bin/bash
# Check which package.json declare semver
rg -nP '"name"\s*:\s*' -C1 code/frameworks/nextjs | sed -n 's/.*"name": "\(.*\)".*/\1/p' | head -n1
echo "Searching for semver dependency declarations..."
rg -nP '"semver"\s*:\s*"' -C2 package.json code/**/package.jsonLength of output: 308
Add semver to dependencies in code/frameworks/nextjs/package.json.
semver is only declared in code/core/package.json; add it under dependencies here so the import resolves correctly.
🤖 Prompt for AI Agents
In code/frameworks/nextjs/src/utils.ts around line 10, the file imports semver
but this package is not declared in code/frameworks/nextjs/package.json; add
"semver" to the dependencies in that package.json (use the same version spec as
declared in code/core/package.json to keep versions consistent), then run your
package manager to update node_modules/lockfile so the import resolves
correctly.
This is an automated pull request that bumps the version from
9.1.10to9.1.11.Once this pull request is merged, it will trigger a new release of version
9.1.11.If you're not a core maintainer with permissions to release you can ignore this pull request.
To do
Before merging the PR, there are a few QA steps to go through:
And for each change below:
This is a list of all the PRs merged and commits pushed directly to
next, that will be part of this release:sb.mockusage in Storybook's deployed in subpaths #32678🍒 Manual cherry picking needed!
The following pull requests could not be cherry-picked automatically because it resulted in merge conflicts.
For each pull request below, you need to either manually cherry pick it, or discard it by replacing the "patch:yes" label with "patch:no" on the PR and re-generate this PR.
git cherry-pick -m1 -x 190788ecb6a54ef3ac8fb730031589dd50f7134agit cherry-pick -m1 -x b0b590227f2b94fba12516ce083c9b02cc63cdb9git cherry-pick -m1 -x a47e297b466f99f97dc4c1fe95df80b42ac4dae0git cherry-pick -m1 -x e9a2317cea0c3b0fb25de5b63ddd6531e547abfbIf you've made any changes doing the above QA (change PR titles, revert PRs), manually trigger a re-generation of this PR with this workflow and wait for it to finish. It will wipe your progress in this to do, which is expected.
Feel free to manually commit any changes necessary to this branch after you've done the last re-generation, following the Make Manual Changes section in the docs, especially if you're making changes to the changelog.
When everything above is done:
Generated changelog
9.1.11
sb.mockusage in Storybook's deployed in subpaths - #32678, thanks valentinpalkovic!Summary by CodeRabbit
New Features
Bug Fixes
Improvements