Skip to content

fix(pages): keep the committed page when a history write fails - #598

Merged
Brooooooklyn merged 1 commit into
mainfrom
fix/navigation-commit-failure
Oct 26, 2026
Merged

Brooooooklyn merged 1 commit into
mainfrom
fix/navigation-commit-failure

Conversation

@Brooooooklyn

Copy link
Copy Markdown
Member

Fixes #562.

A Pages navigation wrote history last. When pushState or replaceState threw (Safari's rate limit, a SecurityError), the new page had already taken over, but the address bar still showed the old URL and the visit never went idle.

before                                   after
──────                                   ─────
render B                                 render B
commit B:                                commit B:
  dispose A, hand over Deferred values     write history ── throws ─┐
  head, title, scroll key → B              (nothing else changed)   │
  write history ── throws                dispose B only            │
                                         show A again  ◄────────────┘
result: page B, URL A, head B,           result: page A, URL A, head A,
        A's Deferred values gone,                visit rejects with the error,
        stuck on "loading"                       navigation goes idle

What changed

navigation-engine.ts   write history first, right after the #507 guard
                       on throw: dispose only the new page, rethrow
scroll-restoration.ts  take the new scroll key only once the entry exists
client.ts              (Vue, Svelte, Solid) put the committed page back
                       when the last visit to settle finds a failed one on screen
react pages-client.ts  on a failed commit: end the visit, dispatch 'fail'
                       (shows the committed page), reject router.visit()
hash visits            a failed write rejects the visit (loading → idle)
                       instead of throwing out of router.visit()

The engine has to write first. Disposing the old page, handing over its Deferred values and updating the head can't be undone, so a rollback after the fact isn't possible.

Overlapping visits:

B renders ── B's write fails      C still loading   → B stays until C settles
                                  C commits         → C shown (normal)
                                  C fails too       → A shown again
hash visit settles while B renders, then B's write fails → A shown again

Kept as before: the #507 guard runs first, #506 (history for every visit that isn't back/forward), and #527/#566 (a same-URL form submission replaces its entry).

Tests

  • pages-client.test.ts, new failed history writes (#562) block. It covers a failed push, a failed replace, a same-URL POST (form state kept), a working visit after a failure, an older visit failing while a newer one loads (the newer one commits, or fails too), a hash visit settling first, a failed hash visit, and the engine called the way React calls it (a second commit() is a no-op, and the old page still gets its streamed Deferred values).
  • pages-scroll.test.ts: after a failed write, positions are saved under the current entry's key.
  • react-pages-client.test.ts (browser): a Link push and a replace Link whose write fails. The page stays, the URL stays, state goes idle, and the next click works.
  • Mutation checks: each source change was put back to main one at a time (engine, core client, the hash-visit-first branch, hash shell, scroll key, React adapter). Each one made a test fail.
  • vp run check, vp lint, vp fmt, vp run test and one full vp test run pass.

Choices to check

  • React useForm has already reported success when the shell arrives, before the commit. If the write then fails, the form keeps its result, the page rolls back, and the error goes to reportError so it isn't dropped silently. Making useForm wait for the commit would change its timing for every submission.
  • The head and title now change after the history entry is written, not before. Browsers ignore the title argument, so this shouldn't show.
  • Not in this PR: Vue, Svelte and Solid <Form>/Link don't catch a rejected visit, so a failed write there is an unhandled rejection (as with any other visit error today).

🤖 Generated with Claude Code

A navigation commit wrote history last. If pushState or replaceState threw
(Safari's rate limit, a SecurityError), the new page had already taken over
the head, scroll key and Deferred values while the address bar still showed
the old URL, and the visit never went idle.

Write history first in the engine commit, so a failed write leaves the
committed page untouched and only disposes the new one. The core client
(Vue, Svelte, Solid) and the React adapter then show the committed page again
and end the visit. A same-page hash visit whose write fails now rejects
through its shell instead of throwing synchronously.

Fixes #562.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Brooooooklyn

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-26T12:59:55.779888Z 14c7102 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 14c7102552

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@cloudflare-workers-and-pages

Copy link
Copy Markdown

🚀 Deploying Preview to Cloudflare 🚀

Preview URL: https://fix-navigation-commit-failure-void-cloud.voidzero-docs.workers.dev (commit 14c7102)

This URL reflects your latest Preview deployment

Preview Deployments by commit

Status Deployment URL Commit Updated (UTC) See this deployment's details
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://3885ed6e-void-cloud.voidzero-docs.workers.dev 14c7102 2026-10-26T12:58:19.274Z Visit the dashboard ↗

@Brooooooklyn
Brooooooklyn merged commit b37d887 into main Oct 26, 2026
20 checks passed
@Brooooooklyn
Brooooooklyn deleted the fix/navigation-commit-failure branch October 26, 2026 13:23
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.

Navigation commit failure leaves the loading state active and causes inconsistent internal state

1 participant