Skip to content

Stop stripping frontend_path twice when matching routes - #7153

Merged
FarhanAliRaza merged 2 commits into
reflex-dev:mainfrom
JohannesGezachew:fix-frontend-path-double-strip
Sep 15, 2026
Merged

FarhanAliRaza merged 2 commits into
reflex-dev:mainfrom
JohannesGezachew:fix-frontend-path-double-strip

Conversation

@JohannesGezachew

@JohannesGezachew JohannesGezachew commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

closes #7090

What

get_router stripped frontend_path from every path it matched, but its two callers pass different kinds of paths:

  • The event handler in app.py matches router_data["pathname"], which the frontend sends relative to frontend_path (see the comment in state.js). With frontend_path="/app", /apple became le and resolved to 404, and /app became / and resolved to the index page.
  • OnLoadInternalState.on_load_internal passes self.router.url.path, which is the browser URL and does include frontend_path. This is why the strip was added to the matcher in fix various issues with frontend_path #5698.

The router now always matches paths relative to frontend_path, and App.get_load_events strips frontend_path from the URL path itself. It strips it only as a whole path segment, so /app/apple still maps to apple, and a URL outside frontend_path such as /apple still gets the 404 load events. The matcher cache is now keyed on the path alone.

Tests

Added to tests/units/test_route.py:

  • test_get_router_ignores_frontend_path: with frontend_path="/app", /apple, /app and / match apple, app and index. On current main the first two fail.

  • test_get_load_events_strips_frontend_path: /app, /app/, /app/apple and /app/app resolve to the right page's load events, and /apple falls back to 404.

  • tests/units/test_route.py: 43 passed.

  • tests/units: 8466 passed. The 207 failures locally are all under tests/units/reflex_cli/v2/ and match what I see on main without this change.

Checklist

  • Bug fix (non-breaking change which fixes an issue)
  • Followed CONTRIBUTING.md; no other open PR for this change
  • Added a news fragment
  • ruff check, ruff format --check and pyright pass on the changed files

The router stripped frontend_path from every path, but the event handler passes the pathname the frontend sends, which is already relative to frontend_path. With frontend_path="/app", /apple resolved to 404 and /app to the index page. Match relative paths in the router and strip frontend_path as a whole segment only in get_load_events, whose path is the browser URL.
@JohannesGezachew
JohannesGezachew requested a review from a team as a code owner September 15, 2026 14:23

@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 tests/units/test_route.py
@greptile-apps

greptile-apps Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the previously reported out-of-prefix routing defect is fully fixed and covered by a regression test.

Summary

This PR makes route matching consistently operate on paths relative to frontend_path.

  • Moves browser URL prefix validation and stripping into App.get_load_events.
  • Rejects browser paths outside the configured frontend prefix.
  • Removes duplicate prefix stripping and configuration-dependent cache keys from get_router.
  • Adds regression coverage for prefixed routes, similarly named routes, the index route, and out-of-prefix URLs.
  • Adds a user-facing bugfix news fragment.

Reviews (2) · Last reviewed commit: "Return 404 load events for URLs outside ..."

Comment thread reflex/app.py Outdated
@codspeed

codspeed Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 40 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing JohannesGezachew:fix-frontend-path-double-strip (9ccab2e) with main (2f63cb3)

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. ↩

A browser URL that does not start with frontend_path is not a page of the app. Keep resolving it to the 404 load events instead of matching it as a relative route.

@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 2 files (changes from recent commits).

Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.

Re-trigger cubic

Comment thread reflex/app.py
@FarhanAliRaza
FarhanAliRaza merged commit 32f5a45 into reflex-dev:main Sep 15, 2026
114 checks passed
FarhanAliRaza added a commit to FarhanAliRaza/reflex that referenced this pull request Sep 30, 2026
reflex/utils/exec.py: kept main's threading import alongside the PR's Callable import; dropped the PR's _match_with_frontend_path wrapper because main (reflex-dev#7153) made get_router match frontend_path-relative paths, which the mount already passes
tests/units/test_app.py: kept the PR's test_page_routes and main's 404 metadata tests
tests/units/utils/test_exec.py: kept the PR's routes-manifest/frontend-mount tests and main's granian tests

Claude-Session: https://claude.ai/code/session_01VDzz2un9mtjmCY24zSGmca
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.

frontend_path is stripped twice for routes beginning with the prefix text

2 participants