Skip to content

Raise actionable error for components declaring library react-router-dom - #6991

Merged
masenf merged 3 commits into
mainfrom
claude/rel-fix-react-router-dom-guard
Aug 28, 2026
Merged

Raise actionable error for components declaring library react-router-dom#6991
masenf merged 3 commits into
mainfrom
claude/rel-fix-react-router-dom-guard

Conversation

@masenf

@masenf masenf commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Defect

FINDING-010 (MEDIUM) from the 0.9.9a1 pre-release testing round: React Router 8 dropped the react-router-dom re-export package and Reflex no longer installs it (#6854), but a custom component declaring library = "react-router-dom" still fell through the generic custom-library install path. bun add then installed an unpinned react-router-dom@^7, nesting a second React Router 7 copy beside the first-party v8:

  • Dev: the component rendered and navigated with no warning (vite dedupes to the hoisted v8 copy), and the stray dependency was persisted into reflex.lock/package.json — working purely by accident.
  • Prod: the build failed at prerender with the cryptic TypeError: Cannot destructure property 'basename' of React10.useContext(...) as it is null, exit 1, with nothing pointing at the real cause.

Fix

Component._get_imports (reflex-base) now checks the component's merged import contributions — library, lib_dependencies, and add_imports alike — and raises a pointered ValueError when any of them resolves to the removed react-router-dom package. The bare name, versioned forms (react-router-dom@7.18.2), and subpath imports (react-router-dom/server, react-router-dom@7.18.2/server) are all caught; similarly named but distinct packages (e.g. react-router-dom-fork) are not. The error names the offending component and tells the author to use library = "react-router" (or "react-router/dom" for RouterProvider/HydratedRouter), matching the migration documented in the React Router 8 changelog entry:

ValueError: The component `BadDomLink` references `react-router-dom`, but React Router 8 removed the `react-router-dom` package and Reflex no longer installs it. Use `library = "react-router"` instead, or `"react-router/dom"` for `RouterProvider`/`HydratedRouter`.

The check sits in the per-component import-collection chokepoint (before the imports cache is populated), so it fires at compile/collection time in both dev and prod, names the component that declared the library, and costs a single startswith pre-filter per import key. News fragments added under news/ and packages/reflex-base/news/.

Test plan

  • New regression tests in tests/units/components/test_component.py:
    • test_get_imports_rejects_react_router_dom_library (parametrized over the bare, versioned, subpath, and versioned+subpath forms) — all 4 fail with DID NOT RAISE ValueError on unfixed main, pass with the fix.
    • test_get_imports_rejects_react_router_dom_lib_dependency — fails on unfixed main, passes with the fix.
    • test_get_imports_allows_react_router_librariesreact-router, react-router/dom, and react-router-dom-fork remain accepted.
  • uv run pytest tests/units/components tests/units/compiler — 4338 passed; uv run pytest tests/units/test_app.py tests/units/reflex_base — 393 passed.
  • uv run ruff check . and uv run ruff format . clean; uv run pyright reflex tests packages/reflex-base — 0 errors (one pre-existing warning in an untouched file).
  • uv run python scripts/make_pyi.py — no pyi_hashes.json changes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x


Generated by Claude Code

Review in cubic

React Router 8 dropped the react-router-dom re-export package and Reflex
no longer installs it (#6854), but a custom component declaring
library="react-router-dom" still fell through the generic custom-library
path: bun installed an unpinned react-router-dom@^7, nesting a second
React Router 7 copy beside the first-party v8. Dev happened to work
(vite deduped to the hoisted v8 copy), while prod builds failed at
prerender with a cryptic "Cannot destructure property 'basename'"
TypeError. Found as FINDING-010 in 0.9.9a1 pre-release testing.

Component._get_imports now rejects any import contribution that
resolves to the react-router-dom package - the bare name, versioned
forms (react-router-dom@7.18.2), and subpath imports
(react-router-dom/server), whether from library, lib_dependencies, or
add_imports - with an error naming the component and pointing at
library="react-router" (or "react-router/dom" for
RouterProvider/HydratedRouter). Similarly named packages such as
react-router-dom-fork are unaffected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x
@masenf
masenf requested a review from a team as a code owner August 28, 2026 18:31
@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR adds compile-time validation that rejects component import contributions resolving to the removed react-router-dom package.

  • Checks merged imports before caching them.
  • Handles bare, versioned, and subpath package forms while allowing similarly named packages.
  • Adds focused regression tests and a bug-fix news fragment.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/reflex-base/src/reflex_base/components/component.py Adds a centralized prohibited-import guard and invokes it on each component’s merged import contributions before caching.
tests/units/components/test_component.py Covers rejected bare, versioned, subpath, and dependency forms, plus accepted replacement and similarly named libraries.
packages/reflex-base/news/+react-router-dom-guard.bugfix.md Documents the new actionable compilation failure and supported React Router replacements.

Reviews (3): Last reviewed commit: "Delete news/+react-router-dom-guard.bugf..." | Re-trigger Greptile

@masenf masenf added this to the v0.9.9 milestone Aug 28, 2026
@codspeed-hq

codspeed-hq Bot commented Aug 28, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 32 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing claude/rel-fix-react-router-dom-guard (b56951a) with main (34bdfc3)

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 4 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/reflex-base/src/reflex_base/components/component.py Outdated
Comment thread news/+react-router-dom-guard.bugfix.md Outdated
Comment thread packages/reflex-base/src/reflex_base/components/component.py Outdated
FarhanAliRaza
FarhanAliRaza previously approved these changes Aug 28, 2026
@masenf
masenf merged commit 0b39d1a into main Aug 28, 2026
111 checks passed
@masenf
masenf deleted the claude/rel-fix-react-router-dom-guard branch August 28, 2026 20:21
masenf pushed a commit that referenced this pull request Aug 28, 2026
…-context-refactor-jv3pig

Picks up #6994, #6986 and #6991. Clean auto-merge; nothing touches client
state, memoization, or foreach.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants