Skip to content

fix(react-router-dom-v6): polish react-router-dom#3125

Open
SoonIter wants to merge 1 commit intomainfrom
syt-vibe-kanban/d4ab-react-router-dom
Open

fix(react-router-dom-v6): polish react-router-dom#3125
SoonIter wants to merge 1 commit intomainfrom
syt-vibe-kanban/d4ab-react-router-dom

Conversation

@SoonIter
Copy link
Copy Markdown
Member

Summary

Related Issue

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

Copilot AI review requested due to automatic review settings February 10, 2026 10:02
@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying rspress-v2 with  Cloudflare Pages  Cloudflare Pages

Latest commit: 083e507
Status:🚫  Build failed.

View logs

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates Rspress SSR/SSG-MD runtime to import StaticRouter via a bundler alias so it works across react-router-dom v6 (server subpath) and v7 (main entry), and wires that alias through the rsbuild/rspack alias resolver.

Changes:

  • Switch SSR/SSG-MD server entries to import StaticRouter from REACT_ROUTER_DOM_SERVER instead of react-router-dom.
  • Extend resolveReactRouterDomAlias() to provide a REACT_ROUTER_DOM_SERVER alias that points to server.mjs for react-router-dom v6.
  • Minor refactor in alias resolution (naming/typing, computed router-dom dir).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
packages/core/src/runtime/ssrServerEntry.tsx Swap StaticRouter import to a new alias-based module ID for SSR compatibility.
packages/core/src/runtime/ssrMdServerEntry.tsx Same alias-based StaticRouter import change for markdown SSR/SSG-MD entry (but introduces a TS directive issue).
packages/core/src/node/utils/reactAlias.ts Add alias mapping for REACT_ROUTER_DOM_SERVER based on detected react-router-dom major version.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -1,5 +1,8 @@
// biome-ignore lint/suspicious/noTsIgnore: bundleless
// @ts-ignore
// @ts-expect-error
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a standalone // @ts-expect-error directive followed by a blank line, so it won’t apply to the import below and will likely trigger TypeScript’s “Unused '@ts-expect-error' directive” error. Remove this directive (or move it to directly precede the specific line that errors with no blank line).

Suggested change
// @ts-expect-error

Copilot uses AI. Check for mistakes.
@@ -1,5 +1,8 @@
// biome-ignore lint/suspicious/noTsIgnore: bundleless
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The biome-ignore lint/suspicious/noTsIgnore comment no longer matches the code (this file now uses @ts-expect-error, not @ts-ignore). Either remove the Biome ignore or update it to the relevant rule to avoid carrying stale lint suppressions.

Suggested change
// biome-ignore lint/suspicious/noTsIgnore: bundleless
// biome-ignore lint/suspicious/noTsExpectError: bundleless

Copilot uses AI. Check for mistakes.
@@ -1,3 +1,5 @@
// @ts-expect-error in v7, StaticRouter is exported from the main entry, but in v6 it's exported from 'react-router-dom/server'
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The @ts-expect-error explanation mentions v6/v7 export differences, but the actual TypeScript error being suppressed here is most likely that REACT_ROUTER_DOM_SERVER is a bundler alias/virtual module that TS can’t resolve. Consider adding an ambient module declaration for REACT_ROUTER_DOM_SERVER (preferred), or update the directive comment to reflect the real error being suppressed.

Suggested change
// @ts-expect-error in v7, StaticRouter is exported from the main entry, but in v6 it's exported from 'react-router-dom/server'
declare module 'REACT_ROUTER_DOM_SERVER' {
export { StaticRouter } from 'react-router-dom/server';
}
// Bundler alias for StaticRouter to handle react-router-dom v6/v7 differences

Copilot uses AI. Check for mistakes.
Comment on lines +57 to +58
// we create a shim that re-exports everything from the main entry plus
// StaticRouter from the server subpath.
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says “we create a shim that re-exports everything from the main entry plus StaticRouter from the server subpath”, but the implementation only creates an alias to server.mjs (no shim/re-export layer). Either implement the described shim, or update the comment to accurately describe the alias behavior so future readers aren’t misled.

Suggested change
// we create a shim that re-exports everything from the main entry plus
// StaticRouter from the server subpath.
// we resolve 'react-router-dom' to its package directory and add a
// separate alias pointing directly to the 'server.mjs' entry when present.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants