Skip to content

useBeforeLeave and scroll restoration break on Back after 50 history entries (_depth taken from the capped history.length) #694

Open

Description

@julien-f

Describe the bug

saveCurrentDepth() (src/lifecycle.ts) stamps a new entry with _depth: window.history.length - 1:

export function saveCurrentDepth() {
  if (!window.history.state || window.history.state._depth == null) {
    window.history.replaceState({ ...window.history.state, _depth: window.history.length - 1 }, "");
  }
  depth = window.history.state._depth;
}

Chromium (kMaxSessionHistoryEntries) and Firefox (browser.sessionhistory.max_entries) cap the session history at 50 entries (WebKit at 100, not tested). Once it is full, pushState drops the oldest entry and history.length stays at 50, so every new entry gets the same _depth as the one below it.

Everything keyed on _depth then misbehaves between those entries:

  • notifyIfNotBlocked (src/lifecycle.ts): Back from the newest entry computes delta = 0, so if (delta && block(delta)) skips block: useBeforeLeave listeners are never called and the navigation can't be prevented.
  • createScrollRestoration (src/routers/scrollRestoration.ts, same path on next): positions are stored per depth, so consecutive entries share one slot, and onPush() deletes every key >= depth. Back restores the offset of the entry just left instead of its own.

Present in 1.0.0 (latest on npm) / main (set() in Router and HashRouter), and on next (2.0.0-next.42) in src/routers/history.ts (saveCurrentDepth, set() in browserHistory() and hashHistory()), where scroll restoration is on by default with the default browser history, so it hits without opting in.

Your Example Website or App

Minimal code below; no hosted repro needed, it shows from the console in any Chromium or Firefox tab.

Steps to Reproduce the Bug or Issue

Fresh tab (so the history starts at length 1):

import { render } from "solid-js/web";
import { Router, Route, useNavigate, useBeforeLeave, useParams } from "@solidjs/router";

function Page() {
  const params = useParams();
  (window as any).nav = useNavigate();
  useBeforeLeave((e) => {
    if ((window as any).guard && !confirm("Leave?")) e.preventDefault();
  });
  return <p>page {params.n}</p>;
}

render(
  () => (
    <Router>
      <Route path="/:n?" component={Page} />
    </Router>
  ),
  document.getElementById("root")!,
);

In the console:

for (let i = 1; i <= 52; i++) {
  nav(`/${i}`);
  await new Promise((r) => setTimeout(r));
  console.log(i, history.length, history.state._depth);
}

Then run guard = true and press the browser Back button.

Expected behavior

  • _depth equals i on every line: the last three print 50, 51, 52.
  • Back triggers the useBeforeLeave confirm.

Actual:

  • From i = 50 on, history.length stays 50 and _depth stays 49 (Firefox: 50, as it applies its cap asynchronously).
  • Back navigates to /51 without the confirm (delta is 0). With <Router scrollRestoration> and a page taller than the window, scrolled before the last push, Back restores the offset of the entry just left.

Below the cap (e.g. Back from /10) the confirm shows and blocking works.

Platform

  • OS: Linux
  • Browser: Chromium 156.0.8078.4, Firefox 157
  • @solidjs/router: 1.0.0 (same code on next 2.0.0-next.42)

Additional context

Suggested fix: stamp each entry the router pushes from the depth of the entry it leaves, rather than from history.length. depth isn't exported from lifecycle.ts, so e.g. a small helper there:

export const nextDepth = () => depth + 1;
// in set() of Router / HashRouter:
window.history.pushState({ ...state, _depth: nextDepth() }, "", value);

and the fallback in saveCurrentDepth() becomes _depth: depth == null ? window.history.length - 1 : depth + 1, so entries pushed outside the router (e.g. location.hash) keep counting up past the cap too.

depth + 1 equals today's value below the cap, including after going Back (pushState drops the forward entries). A Math.max(history.length - 1, depth + 1) variant is wrong there: history.length is read before the forward entries are dropped, so after going Back 3 entries and navigating, the new entry is over-stamped, so a Back that useBeforeLeave prevents still goes through: the listener sees delta -3 instead of -1, and the router's go(3) undo has no forward entries to return to. With the change above, I checked in Chromium and Firefox that Back and Forward past the cap are blocked, scroll is restored, and a reload keeps the stamped depth. replace goes through keepDepth, which the change doesn't touch (from reading the code).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions