-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Next.js: Handle v14 compatibility for draftMode import #33341
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
Next.js: Handle v14 compatibility for draftMode import #33341
Conversation
| import slash from 'slash'; | ||
|
|
||
| import { resolvePackageDir } from '../../../core/src/shared/utils/module'; | ||
|
|
||
| export const loadTemplate = async (name: string, replacements: Record<string, string>) => { | ||
| let template = await fs.readFile( | ||
| join(resolvePackageDir('@storybook/addon-vitest'), 'templates', name), | ||
| 'utf8' | ||
| ); | ||
| Object.entries(replacements).forEach(([key, value]) => (template = template.replace(key, value))); | ||
| // Normalize Windows paths (backslashes) to forward slashes for JavaScript string compatibility | ||
| Object.entries(replacements).forEach( | ||
| ([key, value]) => (template = template.replace(key, slash(value))) | ||
| ); |
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.
This change doesn't relate to the linked issue. Can you remove it and open a new PR that describes what you want to solve?
Fixes storybookjs#32950 The issue occurred because the top-level import of 'next/dist/server/request/draft-mode' doesn't exist in Next.js 14. While webpack aliases were configured to redirect this import to the compatibility shim, the import statement itself was causing build failures before webpack could process it. Changed from a static top-level import to a dynamic require() with try-catch fallback. This allows the webpack alias to resolve the correct location based on Next.js version while gracefully falling back to the Next.js 14 location if needed. - Next.js 14: draftMode is in 'next/dist/client/components/headers' - Next.js 15+: draftMode is in 'next/dist/server/request/draft-mode'
093b603 to
fd9698a
Compare
|
View your CI Pipeline Execution ↗ for commit fd9698a ☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughModifies draftMode resolution in a Next.js headers mock to use runtime try/catch with require() and fallback instead of direct top-level import, avoiding hard build-time dependency. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: 0
🧹 Nitpick comments (1)
code/frameworks/nextjs/src/export-mocks/headers/index.ts (1)
25-26: Consider adding a defensive null-check for clarity and consistency, but the current implementation is functionally sound.The try-catch fallback (lines 17-23) correctly handles Next.js version compatibility. However, if both resolution paths fail,
originalDraftModewould beundefined. While this is unlikely due to webpack alias handling, adding a null-check would be consistent with defensive error handling used elsewhere in the codebase (e.g., navigation/index.ts):if (!originalDraftMode) { throw new Error('Failed to resolve draftMode from Next.js'); } const draftMode = fn(originalDraftMode).mockName('draftMode');This is optional but recommended for better error visibility if the Next.js module structure changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/frameworks/nextjs/src/export-mocks/headers/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/frameworks/nextjs/src/export-mocks/headers/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/frameworks/nextjs/src/export-mocks/headers/index.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/frameworks/nextjs/src/export-mocks/headers/index.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/frameworks/nextjs/src/export-mocks/headers/index.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/frameworks/nextjs/src/export-mocks/headers/index.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Applied to files:
code/frameworks/nextjs/src/export-mocks/headers/index.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/frameworks/nextjs/src/export-mocks/headers/index.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should match the expected return type of the original function
Applied to files:
code/frameworks/nextjs/src/export-mocks/headers/index.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/frameworks/nextjs/src/export-mocks/headers/index.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking without the `spy: true` option in Vitest tests
Applied to files:
code/frameworks/nextjs/src/export-mocks/headers/index.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/frameworks/nextjs/src/export-mocks/headers/index.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/frameworks/nextjs/src/export-mocks/headers/index.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests
Applied to files:
code/frameworks/nextjs/src/export-mocks/headers/index.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/frameworks/nextjs/src/export-mocks/headers/index.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Follow the spy mocking rules defined in `.cursor/rules/spy-mocking.mdc` for consistent mocking patterns with Vitest
Applied to files:
code/frameworks/nextjs/src/export-mocks/headers/index.ts
⏰ 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). (1)
- GitHub Check: normal
🔇 Additional comments (2)
code/frameworks/nextjs/src/export-mocks/headers/index.ts (2)
12-15: Good documentation of the version compatibility issue.The comments clearly explain the Next.js version differences and the rationale for the runtime resolution approach.
16-23: This review comment is based on a misunderstanding of the code's design and webpack alias behavior.The try/catch pattern on lines 16-23 is intentional and correct:
Path resolution is working as designed: Webpack aliases only apply to static
importstatements, notrequire()calls. The code explicitly usesrequire()to avoid the top-level import that would fail on Next.js 14 (where the module doesn't exist). The catch block is not error handling—it's version detection.Fallback validation is not needed: The
fn()function fromstorybook/testgracefully handles undefined values (it's a standard Vitest spy factory). Callingfn(undefined)is safe and works correctly, as shown in the parallel nextjs-vite implementation usingfn(originalDraftMode ?? (headers as any).draftMode).The suggested fix violates coding guidelines: The proposed
console.warn()contradicts the requirement to avoid direct console methods incode/directory files. If logging were needed (it's not), it should usestorybook/internal/node-loggerorstorybook/internal/client-logger.Silent error handling is not an issue: This is not silent error swallowing—it's a silent fallback for version compatibility. The code already includes explanatory comments (lines 12-15) documenting the strategy. Additionally, webpack configuration in
code/frameworks/nextjs/src/aliases/webpack.tsexplicitly ignores the expected warning about missingdraftModeexports, confirming this pattern is intentional.The code is correct as-is. No changes are needed.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 49 | 39 | 🎉 -10 🎉 |
| Self size | 20.52 MB | 20.54 MB | 🚨 +16 KB 🚨 |
| Dependency size | 16.50 MB | 16.41 MB | 🎉 -98 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 127 | 128 | 🚨 +1 🚨 |
| Self size | 1.12 MB | 1.12 MB | 🚨 +36 B 🚨 |
| Dependency size | 21.97 MB | 21.96 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 159 | 160 | 🚨 +1 🚨 |
| Self size | 30 KB | 30 KB | 0 B |
| Dependency size | 23.15 MB | 23.14 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 117 | 118 | 🚨 +1 🚨 |
| Self size | 35 KB | 35 KB | 🚨 +17 B 🚨 |
| Dependency size | 19.76 MB | 19.75 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 183 | 173 | 🎉 -10 🎉 |
| Self size | 775 KB | 775 KB | 🎉 -6 B 🎉 |
| Dependency size | 67.54 MB | 67.46 MB | 🎉 -82 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 176 | 166 | 🎉 -10 🎉 |
| Self size | 30 KB | 30 KB | 🚨 +36 B 🚨 |
| Dependency size | 66.12 MB | 66.03 MB | 🎉 -83 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 50 | 40 | 🎉 -10 🎉 |
| Self size | 999 KB | 1000 KB | 🚨 +217 B 🚨 |
| Dependency size | 37.03 MB | 36.94 MB | 🎉 -83 KB 🎉 |
| Bundle Size Analyzer | node | node |
Next.js: Handle v14 compatibility for draftMode import (cherry picked from commit 4033f33)
Summary
Fixes #32950 - Build errors when using Storybook v10 with Next.js 14
The issue occurred because
next/dist/server/request/draft-modedoesn't exist in Next.js 14, causing build failures. While webpack aliases were configured to handle this version difference, the static top-level import was failing before webpack could apply the aliases.Changes
require()in try-catch blockTechnical Details
Next.js 14:
draftModeis exported fromnext/dist/client/components/headersNext.js 15+:
draftModeis exported fromnext/dist/server/request/draft-modeThe webpack compatibility map already handles this via aliases, but we needed to defer the import to runtime to prevent build-time resolution errors.
Test plan
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.