From bc3d59461080e7d718cf8d42ec1906f6874179c6 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 22 Oct 2026 16:26:50 +0800 Subject: [PATCH] fix(pages): keep a newer visit's URL when an older one finishes later An older visit whose page update was still running could commit after a newer visit had committed. It wrote its URL, head and scroll over the newer page and disposed the newer page's Deferred props (#507). The engine now tracks the newest visit whose URL is in the address bar: the last commit, or a back/forward visit the browser has already moved to. An older commit is skipped, and commit() reports false. - A visit started while a page mounts (for example a redirect) still lets that page commit first, so a replace targets the right entry. - Same-page hash visits do not count, so a page already on screen keeps its URL in step with it. - The core router skips a view transition callback that a newer visit overtook, and settles a skipped visit like an abandoned one: GET resolves undefined, other methods reject with AbortError. React settles the same way. Fixes #507 Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/react/src/runtime/pages-client.ts | 15 +- packages/void/src/pages/client.ts | 15 +- packages/void/src/pages/navigation-engine.ts | 33 ++- packages/void/test/unit/pages-client.test.ts | 277 +++++++++++++++++++ 4 files changed, 330 insertions(+), 10 deletions(-) diff --git a/packages/react/src/runtime/pages-client.ts b/packages/react/src/runtime/pages-client.ts index 3b050f3a4..4cbcceeb2 100644 --- a/packages/react/src/runtime/pages-client.ts +++ b/packages/react/src/runtime/pages-client.ts @@ -437,6 +437,7 @@ function App({ // with the browser URL, so one that renders the query falls back to a client render. React.startTransition(() => setHydrated(true)); } + return true; }, dispose() {}, }), @@ -459,7 +460,7 @@ function App({ committedResources.add(resource.id); resource .commit() - .then(() => { + .then((committed) => { const resolveViewTransition = viewTransitionWaiters.get(resource.id); if (resolveViewTransition) { viewTransitionWaiters.delete(resource.id); @@ -468,7 +469,17 @@ function App({ const visitSettler = pendingVisitSettlers.get(resource.id); if (visitSettler) { pendingVisitSettlers.delete(resource.id); - visitSettler.resolve(result.pageData); + if (committed) { + visitSettler.resolve(result.pageData); + } else if (resource.method.toUpperCase() === 'GET') { + // A newer visit committed first. Settle like any abandoned visit. + visitSettler.resolve(undefined); + } else { + visitSettler.reject(new DOMException('Navigation aborted', 'AbortError')); + } + } + if (!committed) { + return; } if (activePendingNavigationId === resource.id) { activePendingNavigationId = null; diff --git a/packages/void/src/pages/client.ts b/packages/void/src/pages/client.ts index ce9a42f54..c2f27094e 100644 --- a/packages/void/src/pages/client.ts +++ b/packages/void/src/pages/client.ts @@ -357,15 +357,26 @@ export async function createVoidRouter(config: VoidRouterConfig): Promise<{ preserveState: resource.options.preserveState ?? resource.method !== 'GET', }); + let applied = true; const useTransition = resource.options.viewTransition ?? config.viewTransitions; if (useTransition && document.startViewTransition) { - const transition = document.startViewTransition(() => applyUpdate()); + // The callback runs on a later frame. A newer visit started by then renders instead. + const transition = document.startViewTransition(() => { + if (activeNavigationId !== resource.id) { + applied = false; + return; + } + return applyUpdate(); + }); await transition.updateCallbackDone; } else { await applyUpdate(); } - await resource.commit(); + // A newer visit can commit while this page is being applied; its URL stays. + if (!applied || !(await resource.commit())) { + throw new DOMException('Navigation aborted', 'AbortError'); + } if (activeNavigationId === resource.id) { setRouteState(currentUrl(), result.pageData.params ?? {}); } diff --git a/packages/void/src/pages/navigation-engine.ts b/packages/void/src/pages/navigation-engine.ts index 217981c09..4499cc572 100644 --- a/packages/void/src/pages/navigation-engine.ts +++ b/packages/void/src/pages/navigation-engine.ts @@ -104,7 +104,8 @@ export type PageNavigationResource = { immediate: boolean; shell: Promise; abort(): void; - commit(): Promise; + /** Resolves false when a newer visit has already set the URL, so nothing was committed. */ + commit(): Promise; dispose(): void; }; @@ -252,6 +253,9 @@ export async function createPageNavigationEngine( const _prefetchCache = new PrefetchCache(); let _nextNavigationId = 0; + // The newest visit whose URL is in the address bar: the last commit, or a back/forward + // visit, which the browser has already moved to. An older visit must not commit over it. + let _urlNavigationId = 0; let _pendingResource: PageNavigationResource | null = null; let _committedResource: PageNavigationResource | null = null; let _committedDeferredRegistry: DeferredRegistry | null = null; @@ -853,17 +857,23 @@ export async function createPageNavigationEngine( deferredRegistry: DeferredRegistry, resourceId: number, getResource: () => PageNavigationResource, - ): () => Promise { + ): () => Promise { let committed = false; return async () => { if (committed) { - return; + return false; } committed = true; const result = await shell; if (!result) { - return; + return false; + } + // A newer visit committed while this one was being applied. Committing now would put + // back this page's URL, head, scroll and Deferred ownership over the newer page. + if (resourceId < _urlNavigationId) { + return false; } + _urlNavigationId = resourceId; if (_pendingResource?.id === resourceId) { _pendingResource = null; } @@ -922,6 +932,7 @@ export async function createPageNavigationEngine( restoreScroll(options._scrollPosition, commitTarget.hash); } }); + return true; }; } @@ -993,11 +1004,21 @@ export async function createPageNavigationEngine( immediate: true, shell: Promise.resolve(undefined), abort() {}, - async commit() {}, + // Same-page hash visits commit right away. They do not stop an older visit + // whose page is already on screen from committing its URL. + async commit() { + return true; + }, dispose() {}, }; } + if (options._popstate) { + // The browser has already moved to this entry. A same-page hash entry is left out + // above: it renders nothing, so an older page on screen still commits its URL. + _urlNavigationId = id; + } + const controller = new AbortController(); const deferredRegistry = new DeferredRegistry(); const commitTarget = { fetchUrl: effectiveFetchUrl, hash }; @@ -1012,7 +1033,7 @@ export async function createPageNavigationEngine( deferredRegistry, ); - let commitResource: () => Promise = async () => {}; + let commitResource: () => Promise = async () => false; const stop = () => { controller.abort(abortError()); deferredRegistry.dispose(); diff --git a/packages/void/test/unit/pages-client.test.ts b/packages/void/test/unit/pages-client.test.ts index dad6e8e18..f9a159836 100644 --- a/packages/void/test/unit/pages-client.test.ts +++ b/packages/void/test/unit/pages-client.test.ts @@ -1350,3 +1350,280 @@ describe('preserveState history updates (#506)', () => { expect(history.replaceState).not.toHaveBeenCalled(); }); }); + +describe('superseded navigation commits (#507)', () => { + function gate() { + let release!: () => void; + const promise = new Promise((resolve) => { + release = resolve; + }); + return { promise, release }; + } + + // pushState and replaceState move window.location, like a browser. + function setup() { + const documentStub = stubClientGlobals(); + Object.assign(documentStub, { title: 'Home' }); + const writeHistory = (_state: unknown, _title: string, url?: string | URL | null) => { + if (url != null) { + const target = new URL(url, window.location.origin); + Object.assign(window.location, { + pathname: target.pathname, + search: target.search, + hash: target.hash, + }); + } + }; + vi.mocked(history.pushState).mockImplementation(writeHistory); + vi.mocked(history.replaceState).mockImplementation(writeHistory); + const fetchGates = new Map>(); + vi.stubGlobal( + 'fetch', + vi.fn(async (url: string) => { + await fetchGates.get(url); + return url === '/a' + ? pageJson({ component: 'a', url, props: {}, head: { title: 'A' } }) + : pageJson({ + component: 'b', + url, + props: { stats: null }, + deferredKeys: { stats: 'stats' }, + head: { title: 'B' }, + }); + }), + ); + const updateGates = new Map>(); + const rendered: Array = []; + const applyUpdate = vi.fn((pageData: { url?: string }) => { + rendered.push(pageData.url ?? ''); + return updateGates.get(pageData.url ?? ''); + }); + const rejectDeferred = vi.fn((_ref, error) => error); + return { documentStub, fetchGates, updateGates, rendered, applyUpdate, rejectDeferred }; + } + + async function router(page: ReturnType, config: TestRouterConfig = {}) { + const created = await createTestRouter({ + ...config, + adapter: { + applyUpdate: page.applyUpdate, + createDeferred: () => ({ loading: true }), + isLoading: () => true, + rejectDeferred: page.rejectDeferred, + }, + components: { a: loadComponent, b: loadComponent, home: loadComponent }, + matchRoute: (url) => (url.startsWith('/a') ? 'a' : url.startsWith('/b') ? 'b' : 'home'), + }); + vi.mocked(history.pushState).mockClear(); + vi.mocked(history.replaceState).mockClear(); + vi.mocked(window.scrollTo).mockClear(); + return created.router; + } + + async function entered(page: ReturnType, url: string) { + await vi.waitFor(() => expect(page.rendered).toContain(url)); + } + + it('commits each visit in order when they do not overlap', async () => { + const page = setup(); + const r = await router(page); + + await r.visit('/a'); + await r.visit('/b'); + + expect(vi.mocked(history.pushState).mock.calls.map((call) => call[2])).toEqual(['/a', '/b']); + expect(window.location.pathname).toBe('/b'); + expect(page.documentStub).toMatchObject({ title: 'B' }); + }); + + it('keeps the newer page when an older visit finishes applying after it commits', async () => { + const page = setup(); + const a = gate(); + page.updateGates.set('/a', a.promise); + const r = await router(page); + + const visitA = r.visit('/a'); + await entered(page, '/a'); + await r.visit('/b'); + vi.mocked(window.scrollTo).mockClear(); + a.release(); + + await expect(visitA).resolves.toBeUndefined(); + expect(window.location.pathname).toBe('/b'); + expect(r.url.pathname).toBe('/b'); + expect(vi.mocked(history.pushState).mock.calls.map((call) => call[2])).toEqual(['/b']); + expect(page.documentStub).toMatchObject({ title: 'B' }); + expect(window.scrollTo).not.toHaveBeenCalled(); + // A's commit would have disposed B and rejected its pending Deferred prop. + expect(page.rejectDeferred).not.toHaveBeenCalled(); + }); + + it('commits an older visit that finishes before the newer one commits', async () => { + const page = setup(); + const a = gate(); + const b = gate(); + page.updateGates.set('/a', a.promise); + page.fetchGates.set('/b', b.promise); + const r = await router(page); + + const visitA = r.visit('/a'); + await entered(page, '/a'); + const visitB = r.visit('/b'); + a.release(); + // A is on screen, so its URL is written until B commits. + await expect(visitA).resolves.toMatchObject({ url: '/a' }); + expect(window.location.pathname).toBe('/a'); + + b.release(); + await visitB; + expect(vi.mocked(history.pushState).mock.calls.map((call) => call[2])).toEqual(['/a', '/b']); + expect(window.location.pathname).toBe('/b'); + }); + + it('lets a visit started while the new page mounts replace that page', async () => { + const page = setup(); + const r = await router(page); + // Like a redirect from Vue's onMounted, which runs before applyUpdate settles. + page.applyUpdate.mockImplementation((pageData: { url?: string }) => { + page.rendered.push(pageData.url ?? ''); + if (pageData.url === '/a') { + void r.visit('/b', { replace: true }); + } + return Promise.resolve(); + }); + + await r.visit('/a'); + await vi.waitFor(() => expect(window.location.pathname).toBe('/b')); + + expect(vi.mocked(history.pushState).mock.calls.map((call) => call[2])).toEqual(['/a']); + expect(vi.mocked(history.replaceState).mock.calls.at(-1)?.[2]).toBe('/b'); + }); + + it('keeps page and URL together when a same-page hash visit starts during an update', async () => { + const page = setup(); + const a = gate(); + page.updateGates.set('/a', a.promise); + const r = await router(page); + + const visitA = r.visit('/a'); + await entered(page, '/a'); + await r.visit('#x'); + a.release(); + + await expect(visitA).resolves.toMatchObject({ url: '/a' }); + expect(window.location.pathname).toBe('/a'); + expect(page.documentStub).toMatchObject({ title: 'A' }); + }); + + it('does not push an older URL after a back/forward visit', async () => { + const page = setup(); + const a = gate(); + const b = gate(); + page.updateGates.set('/a', a.promise); + page.fetchGates.set('/b', b.promise); + const r = await router(page); + + const visitA = r.visit('/a'); + await entered(page, '/a'); + // The browser has already moved to /b when popstate fires, before /b loads. + window.location.pathname = '/b'; + const visitB = r.visit('/b', { _popstate: true, _scrollPosition: null }); + a.release(); + await expect(visitA).resolves.toBeUndefined(); + b.release(); + await visitB; + + expect(history.pushState).not.toHaveBeenCalled(); + expect(window.location.pathname).toBe('/b'); + }); + + it('keeps page and URL together after Back to a same-page hash entry during an update', async () => { + const page = setup(); + const a = gate(); + page.updateGates.set('/a', a.promise); + const r = await router(page); + await r.visit('#s'); + + const visitA = r.visit('/a'); + await entered(page, '/a'); + // Back from /#s to /: a same-page entry, so nothing new renders. + window.location.hash = ''; + await r.visit('/', { _popstate: true, _scrollPosition: null }); + a.release(); + + await expect(visitA).resolves.toMatchObject({ url: '/a' }); + expect(window.location.pathname).toBe('/a'); + expect(page.documentStub).toMatchObject({ title: 'A' }); + }); + + it('rejects a superseded form submission with an AbortError', async () => { + const page = setup(); + const a = gate(); + page.updateGates.set('/a', a.promise); + const r = await router(page); + + const submitA = r.visit('/a', { method: 'post', data: { name: 'x' } }); + await entered(page, '/a'); + await r.visit('/b'); + a.release(); + + await expect(submitA).rejects.toMatchObject({ name: 'AbortError' }); + expect(window.location.pathname).toBe('/b'); + }); + + it('does not apply an older view transition after a newer visit rendered', async () => { + const page = setup(); + const callbacks: Array<() => unknown> = []; + Object.assign(document, { + startViewTransition(callback: () => unknown) { + const updateCallbackDone = new Promise((resolve) => { + callbacks.push(async () => { + await callback(); + resolve(); + }); + }); + return { updateCallbackDone }; + }, + }); + const r = await router(page); + + const visitA = r.visit('/a', { viewTransition: true }); + await vi.waitFor(() => expect(callbacks).toHaveLength(1)); + await r.visit('/b', { viewTransition: false }); + await callbacks[0](); + + await expect(visitA).resolves.toBeUndefined(); + expect(page.rendered).toEqual(['/b']); + expect(window.location.pathname).toBe('/b'); + }); + + it('skips an older engine commit after a newer one, as the React adapter calls it', async () => { + const page = setup(); + const engine = await createPageNavigationEngine({ + adapter: { + createDeferred: () => ({ loading: true }), + isLoading: () => true, + resolveDeferred: (_ref, value) => value, + rejectDeferred: page.rejectDeferred, + }, + components: { a: loadComponent, b: loadComponent, home: loadComponent }, + layoutTree: {}, + routeMeta: {}, + matchRoute: (url) => (url.startsWith('/a') ? 'a' : 'b'), + viewTransitions: false, + prefetchHoverDelay: 75, + prefetchDefaultCacheFor: parseCacheFor('30s'), + }); + + const a = engine.prepareVisit('/a'); + await a.shell; + const b = engine.prepareVisit('/b'); + await b.shell; + await b.commit(); + await a.commit(); + + expect(vi.mocked(history.pushState).mock.calls.map((call) => call[2])).toEqual(['/b']); + expect(page.documentStub).toMatchObject({ title: 'B' }); + expect(page.rejectDeferred).not.toHaveBeenCalled(); + }); +});