Repository navigation
[DO NOT MERGE YET] Navigation API stand-in — stopgap pending remix-run/remix#11641 - #924
Closed
kentcdodds wants to merge 1 commit into
Closed
kentcdodds wants to merge 1 commit into
kentcdodds wants to merge 1 commit into
Conversation
Remix 3 beta's client runtime reads window.navigation.updateCurrentEntry()
unguarded while it boots (@remix-run/ui's startNavigationListener), so
run() throws on every browser without the Navigation API. That API only
reached Firefox in 147 and WebKit in 26.2, both in early 2026, so it
still covers Firefox 146 and below plus every browser on iOS 26.1 and
below.
The throw lands after createFrame() and before run() returns, so
hydration already in flight survives and the app stays broadly usable.
The real cost is an unhandled TypeError on every page load plus the
app.addEventListener('error', ...) wiring in entry.tsx that never runs,
so affected users report no client errors at all (KODY-CLOUDFLARE-17).
Kody never uses the Navigation API - client-router.tsx drives SPA
navigation through history.pushState and a document click handler - so a
stand-in that never emits 'navigate' behaves like the real thing here.
Only the members the Remix runtime reads are implemented.
This is a stopgap for remix-run/remix#11641 and should not merge while
the upstream fix is still in play.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a minimal Navigation API fallback, installs it during worker client startup, and adds unit and Playwright coverage for hydration and client-side navigation when ChangesNavigation API compatibility
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant WorkerClient
participant NavigationFallback
participant ClientRouter
Browser->>WorkerClient: Start application without window.navigation
WorkerClient->>NavigationFallback: Install fallback
WorkerClient->>ClientRouter: Start runtime
ClientRouter->>NavigationFallback: Read navigation state and navigate
NavigationFallback->>Browser: Assign or replace destination
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
kody-bot
marked this pull request as ready for review
July 24, 2026 21:35
Contributor
|
🔎 Preview deployed: https://kody-pr-924.kody-a99.workers.dev Worker: Mocks:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of #921. The other half of that PR — our own update-during-render bug, KODY-CLOUDFLARE-10 — was merged separately.
The bug this works around
@remix-run/ui'sstartNavigationListenerreadswindow.navigation.updateCurrentEntry()with no feature check, andrun()calls it during boot:Why this is worth your attention despite a 4-event count
Affected users report nothing at all. The throw happens inside
run(), sorun()never returns — which meansentry.tsxnever reaches the two lines after it:So for exactly the browsers hitting this, our client-side error reporting is never wired up. Every client error those users encounter — this one and any other — goes unreported. The event count is not a measure of how many users are affected; it is a measure of how thoroughly the bug hides itself.
Who is affected
The Navigation API is far newer than it looks. It reached Firefox in 147 and WebKit in 26.2, both in early 2026:
Every browser on iOS is WebKit, so on iOS ≤ 26.1 this hits Safari, Chrome for iOS, and in-app browsers alike. The Sentry group matches exactly: Firefox 141 on Ubuntu, Chrome Mobile iOS 150 on iOS 26.1, and the X in-app browser on iOS 26.1.
What actually breaks (corrected)
I initially assumed hydration died completely. Testing against a real Firefox 146 disproved that, and I would rather you have the accurate version: the throw lands after
createFrame(...), so hydration already in flight survives and the app stays broadly usable — client-side routing still intercepts link clicks on production today.The concrete costs are:
TypeErroron every page load for affected visitors.navigateinterception never installs (we do not rely on it — see below).I posted the same correction on the upstream issue.
What the stopgap does
Kody never uses the Navigation API.
client-router.tsxdrives SPA navigation withhistory.pushStateand a document-level click handler, and it callspreventDefault()before the Remix listener could ever intercept. So a stand-in that never emitsnavigateis behaviourally identical to the real API for this app, and it is a strict no-op wherever the real API exists (if (win.navigation) return false).Only the members the Remix runtime reads are implemented —
updateCurrentEntry,entries,navigate, plusEventTarget. It is one self-contained file plus two lines inentry.tsx, removable in a single revert once upstream lands.The case against merging it
window.navigationwhere the browser deliberately does not is the kind of thing that can confuse other feature-detecting code, even though nothing in our bundle does that today (the Sentry SDK uses history patching, not the Navigation API).The case for merging it as an interim
Verification
Playwright ships Firefox 146.0.1, the last release without the Navigation API, so this is the genuine failure condition in a real Gecko engine rather than a simulation. Production (no fix) versus a preview with the fix, same page, same minute:
Also covered by:
packages/worker/client/navigation-api-fallback.node.test.ts— the fallback contract, and that a real Navigation API is never replaced.e2e/ssr-hydration.spec.ts— a Chromium test that shadowswindow.navigationand asserts the app still hydrates and handles SPA navigation, with awindowmarker proving the client router took the click rather than the browser doing a document load.npm run validatepasses.System recap — extends existing primitives (medium risk — stopgap, not for merge)
Mode: recap · Base:
main@569128cb· Head:2b6b4970Classification: extends — no new primitives;
app-ui's boot sequence gains afeature-detected global stand-in installed before the Remix runtime starts.
Primitives touched
app-uiwindow.navigationwhen the browser lacks itSystem map
Client boot feature-detects the Navigation API and installs a stand-in before starting the Remix runtime, so
run()completes andentry.tsxgets to wire up its error listener.Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
flowchart LR appUi["app-ui<br/>Browser app (Remix 3)"]:::extended remixRuntime["remix-ui<br/>Remix client runtime"]:::untouched sentryTunnel["sentry-tunnel<br/>Same-origin error reporting"]:::untouched appUi -->|"installNavigationApiFallback(window) before run()"| remixRuntime remixRuntime -->|"run() returns, so app.addEventListener('error') is registered"| appUi appUi -->|"client errors reach Sentry on affected browsers again"| sentryTunnel classDef touched fill:#1a7f37,color:#fff classDef extended fill:#9a6700,color:#fff classDef added fill:#cf222e,color:#fff classDef untouched fill:#57606a,color:#fffBefore / after
installNavigationApiFallbackreturns earlyTypeErrorper page load;entry.tsxnever wires up error reportingrun()returns and error reporting is wired upInvariants
No change to per-user isolation. The stand-in holds only the current entry's navigation state and no user data.
Summary by CodeRabbit
New Features
Bug Fixes