Next.js: Fix Link component override in appDirectory configuration#31251
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM
3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
|
View your CI Pipeline Execution ↗ for commit f9534d8.
☁️ Nx Cloud last updated this comment at |
valentinpalkovic
added
the
patch:yes
|
Hi @yatishgoel, Thank you for opening the PR! Could you take a look at the build pipeline? Unfortunately, it is failing. |
|
Superseded by #32131 |
|
@valentinpalkovic – seeing as #32131 did not actually address the issue, should this PR be re-evaluated? |
valentinpalkovic
changed the title
|
@yatishgoel Could you merge |
📝 WalkthroughWalkthroughThis change expands the Next.js Storybook integration's public API by adding a new mock export for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
code/frameworks/nextjs/package.json (1)
199-222: Bundler entries for link mock and compat modules look correct; clarify export surfaceThe three new bundler entries
./src/export-mocks/link/index.tsx./src/compatibility/segment.compat.ts./src/compatibility/redirect-status-code.compat.tsline up with the new webpack aliases and compatibility helpers and ensure those files are built into
dist. That’s what you need so/dist/export-mocks/link/indexactually exists for the alias inexport-mocks/webpack.ts.One thing to double-check:
- If
segment.compat/redirect-status-code.compatand the link mock are meant to be publicly importable (beyond internal use), they should also have matchingexports(and ideallytypesVersions) entries, following the existing pattern used fordraft-mode.compatand the other*.mockmodules.- If they’re purely internal helpers, the current bundler-only entries are fine; just keep their usage via internal relative imports to avoid surprising consumers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
code/frameworks/nextjs/package.json(1 hunks)code/frameworks/nextjs/src/export-mocks/link/index.tsx(1 hunks)code/frameworks/nextjs/src/export-mocks/webpack.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/link/index.tsxcode/frameworks/nextjs/src/export-mocks/webpack.tscode/frameworks/nextjs/package.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/frameworks/nextjs/src/export-mocks/link/index.tsxcode/frameworks/nextjs/src/export-mocks/webpack.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/link/index.tsxcode/frameworks/nextjs/src/export-mocks/webpack.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/link/index.tsxcode/frameworks/nextjs/src/export-mocks/webpack.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/link/index.tsxcode/frameworks/nextjs/src/export-mocks/webpack.tscode/frameworks/nextjs/package.json
🧠 Learnings (15)
📓 Common learnings
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
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: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
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: mrginglymus
Repo: storybookjs/storybook PR: 32556
File: code/core/package.json:309-313
Timestamp: 2025-09-29T13:20:23.346Z
Learning: The `fast-printf` dependency in Storybook's core package is bundled into the final distribution during the build process, so it should remain in devDependencies rather than being moved to dependencies, following the same pattern as other bundled dependencies like `open`.
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Follow existing patterns and conventions in the Storybook codebase
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
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
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
📚 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/link/index.tsxcode/frameworks/nextjs/src/export-mocks/webpack.tscode/frameworks/nextjs/package.json
📚 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} : Group related mocks together in Vitest tests
Applied to files:
code/frameworks/nextjs/src/export-mocks/link/index.tsxcode/frameworks/nextjs/package.json
📚 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/**/*.{ts,tsx,js,jsx} : Export functions that need to be tested from their modules
Applied to files:
code/frameworks/nextjs/src/export-mocks/link/index.tsxcode/frameworks/nextjs/package.json
📚 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/link/index.tsx
📚 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/link/index.tsxcode/frameworks/nextjs/src/export-mocks/webpack.tscode/frameworks/nextjs/package.json
📚 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/link/index.tsxcode/frameworks/nextjs/package.json
📚 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 inline mock implementations within test cases in Vitest tests
Applied to files:
code/frameworks/nextjs/src/export-mocks/link/index.tsxcode/frameworks/nextjs/package.json
📚 Learning: 2025-11-05T09:38:47.712Z
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.
Applied to files:
code/frameworks/nextjs/src/export-mocks/link/index.tsxcode/frameworks/nextjs/package.json
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/frameworks/nextjs/src/export-mocks/webpack.tscode/frameworks/nextjs/package.json
📚 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/webpack.tscode/frameworks/nextjs/package.json
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/frameworks/nextjs/package.json
📚 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 direct function mocking without `vi.mocked()` in Vitest tests
Applied to files:
code/frameworks/nextjs/package.json
📚 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/package.json
📚 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} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/frameworks/nextjs/package.json
🧬 Code graph analysis (1)
code/frameworks/nextjs/src/export-mocks/link/index.tsx (1)
code/core/src/test/spy.ts (1)
fn(36-39)
🔇 Additional comments (1)
code/frameworks/nextjs/src/export-mocks/webpack.ts (1)
7-19: Link aliases match existing mock patternThe new aliases for
'next/link'and'@storybook/nextjs/link.mock'are consistent with the existing cache/headers/navigation/router mappings and correctly point at the new dist module. Just make sure the built artifact continues to live at/dist/export-mocks/link/index.{mjs,js}so these stay in sync with the bundler output.
| import React from 'react'; | ||
|
|
||
| import { fn } from 'storybook/test'; | ||
|
|
||
| // Mock implementation for next/link | ||
| const mockLink = fn().mockName('next/link::Link'); | ||
|
|
||
| const linkExports = { | ||
| default: mockLink, | ||
| }; | ||
|
|
||
| export default linkExports.default; | ||
| export { mockLink as Link }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Let me verify the React import concern and linkExports object pattern one more time
cat << 'EOF' > /tmp/verify.ts
// From the snippet in review comment:
import React from 'react';
import { fn } from 'storybook/test';
const mockLink = fn().mockName('next/link::Link');
const linkExports = { default: mockLink };
export default linkExports.default;
export { mockLink as Link };
// Analysis:
// 1. React import: used? No - verified
// 2. linkExports: necessary? No - it just wraps mockLink, verified
// 3. fn() behavior: is it a function that returns undefined?
// - fn() is Vitest spy, when called returns undefined by default
// - When used as React component like <Link />, React would not render it properly
EOF
cat /tmp/verify.tsRepository: storybookjs/storybook
Length of output: 186
Remove unused React import and simplify mock export structure
The React import is unused (no JSX or React APIs called), and the linkExports object unnecessarily wraps mockLink. Both can be removed by exporting mockLink directly:
-import React from 'react';
-
-import { fn } from 'storybook/test';
+import { fn } from 'storybook/test';
// Mock implementation for next/link
const mockLink = fn().mockName('next/link::Link');
-
-const linkExports = {
- default: mockLink,
-};
-
-export default linkExports.default;
+export default mockLink;
export { mockLink as Link };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import React from 'react'; | |
| import { fn } from 'storybook/test'; | |
| // Mock implementation for next/link | |
| const mockLink = fn().mockName('next/link::Link'); | |
| const linkExports = { | |
| default: mockLink, | |
| }; | |
| export default linkExports.default; | |
| export { mockLink as Link }; | |
| import { fn } from 'storybook/test'; | |
| // Mock implementation for next/link | |
| const mockLink = fn().mockName('next/link::Link'); | |
| export default mockLink; | |
| export { mockLink as Link }; |
🤖 Prompt for AI Agents
In code/frameworks/nextjs/src/export-mocks/link/index.tsx around lines 1 to 13,
remove the unused "import React from 'react';" and eliminate the unnecessary
linkExports object by exporting mockLink directly as the default and named
export; specifically delete the React import and replace the linkExports-based
default export with a direct default export of mockLink while keeping the named
export "Link" mapped to mockLink.
ndelangen
added
patch:yes
yatishgoel
force-pushed
the
fix/nextjs15-link-component-override
branch
from
f9534d8 to
b3e8ea8
Compare
|
Thanks for prioritizing this! Let me know if anything else is needed. |
|
Hi @yatishgoel Thank you for resolving the merge conflicts. Could you please take a look into the build errors for |
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
…ating package.json to include new export mappings for link mock and related images.
|
Hi @yatishgoel the nextjs Link story is broken. Empty links are rendered.
|
|
The empty links are happening because the mock uses I'm planning to replace it with a One tradeoff is that the mock would no longer use |
|
Can't we just create an |
valentinpalkovic
moved this from Empathy Queue (prioritized)
to On Hold
in Core Team Projects
…d improve click handling. The MockLink component now constructs href strings from object types and prevents default link behavior while invoking the mock action.
… to ensure proper functionality of the mock action.
…-override Next.js: Fix Link component override in appDirectory configuration (cherry picked from commit 40e556a)
github-actions
Bot
added
the
patch:done
|
I believe this may not work correctly for Next.js links with |
| return ( | ||
| <a ref={ref} href={hrefString} onClick={handleClick} {...rest}> | ||
| {children} | ||
| </a> | ||
| ); |
There was a problem hiding this comment.
I think if legacyBehavior is true, this should pass the props onto the child instead of rendering an <a> itself. #33861
5 participants
Closes #30390
What I did
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
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 canary-release-pr.yml --field pr=<PR_NUMBER>Greptile Summary
Added mock implementation and webpack alias mappings for Next.js Link component to fix navigation issues in Next.js 15 with appDirectory configuration.
code/frameworks/nextjs/src/export-mocks/link/index.tsxwith mock Link component implementationcode/frameworks/nextjs/src/export-mocks/webpack.tsfor proper aliasingcode/frameworks/nextjs/package.jsonexports to include new Link mock componentThe changes ensure proper Link component behavior in stories by preventing navigation to non-existent pages when using Next.js 15 in appDirectory mode.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.