Skip to content

Commit 7bcdbdf

Browse files
cursoragent0froq
andcommitted
fix(pages): replace history for same-page action re-renders
Fixes #527 ## Summary Named actions no longer pollute history when they re-render the current page. #530 already removed `?actionName` from the committed URL; #531 still used `pushState` for every mutation, so each `useForm('/db?create')` / `action()` submit added another back-stack entry at `/db`. - Same-document mutations use `replaceState` instead of `pushState`. - `mutationCommitUrl()` prefers the server `pageData.url` when the action updates query params on the same path (e.g. `POST /books?delete` → `/books?page=2` still pushes once). ## Tests Updated POST history expectations to `replaceState` when the browser URL already matches the committed page URL. Added coverage for URL-changing POSTs and repeated same-page named-action submits. `vp test run packages/void/test/unit/pages-client.test.ts` — 69/69 passed. Co-authored-by: froQ <sayhola@froq.me>
1 parent 6d0cbf4 commit 7bcdbdf

2 files changed

Lines changed: 65 additions & 4 deletions

File tree

‎packages/void/src/pages/navigation-engine.ts‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,22 @@ function withoutActionKey(fetchUrl: string, pageUrl: string): string {
2626
: fetchUrl;
2727
}
2828

29+
/** URL to commit after a page mutation (strip action keys, then prefer the server page URL). */
30+
function mutationCommitUrl(fetchUrl: string, pageUrl?: string): string {
31+
if (!pageUrl) {
32+
return withoutActionKey(fetchUrl, '');
33+
}
34+
const withoutKey = withoutActionKey(fetchUrl, pageUrl);
35+
if (withoutKey !== fetchUrl) {
36+
return withoutKey;
37+
}
38+
const fetchPathEnd = fetchUrl.indexOf('?');
39+
const fetchPath = fetchPathEnd === -1 ? fetchUrl : fetchUrl.slice(0, fetchPathEnd);
40+
const pagePathEnd = pageUrl.indexOf('?');
41+
const pagePath = pagePathEnd === -1 ? pageUrl : pageUrl.slice(0, pagePathEnd);
42+
return fetchPath === pagePath ? pageUrl : fetchUrl;
43+
}
44+
2945
export type PageComponentModule = {
3046
default: unknown;
3147
};
@@ -835,7 +851,7 @@ export async function createPageNavigationEngine(
835851

836852
if (method !== 'GET') {
837853
deferredRegistry.preserveValuesFrom(_committedDeferredRegistry, pageData);
838-
commitTarget.fetchUrl = withoutActionKey(fetchUrl, pageData.url);
854+
commitTarget.fetchUrl = mutationCommitUrl(fetchUrl, pageData.url);
839855
}
840856

841857
const [component, layouts] = await Promise.race([
@@ -923,7 +939,12 @@ export async function createPageNavigationEngine(
923939
// preserveState only controls component state; back/forward traversal already moved history.
924940
if (!options._popstate) {
925941
const historyUrl = commitTarget.fetchUrl + commitTarget.hash;
926-
scrollRestoration.writeHistory(historyUrl, !!options.replace);
942+
const resource = getResource();
943+
const currentDocumentUrl = window.location.pathname + window.location.search;
944+
const replaceHistory =
945+
!!options.replace ||
946+
(resource.method !== 'GET' && historyUrl === currentDocumentUrl + commitTarget.hash);
947+
scrollRestoration.writeHistory(historyUrl, replaceHistory);
927948
}
928949
scrollRestoration.commit();
929950

‎packages/void/test/unit/pages-client.test.ts‎

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -514,8 +514,11 @@ describe('createVoidRouter', () => {
514514
['/books?delete', '/books', '/books'],
515515
['/books?page=2', '/books?page=2', '/books?page=2'],
516516
['/books?a&b', '/books?a&b', '/books?a&b'],
517-
])('writes POST %s with page URL %s to history as %s', async (url, pageUrl, historyUrl) => {
517+
])('replaces history for POST %s with page URL %s as %s', async (url, pageUrl, historyUrl) => {
518518
stubClientGlobals();
519+
const historyLocation = new URL(historyUrl, 'http://localhost');
520+
window.location.pathname = historyLocation.pathname;
521+
window.location.search = historyLocation.search;
519522
vi.stubGlobal(
520523
'fetch',
521524
vi.fn(async () => pageJson({ component: 'home', props: {}, url: pageUrl, errors: null })),
@@ -524,7 +527,44 @@ describe('createVoidRouter', () => {
524527

525528
await router.visit(url, { method: 'POST' });
526529

527-
expect(history.pushState).toHaveBeenCalledWith(expect.anything(), '', historyUrl);
530+
expect(history.replaceState).toHaveBeenCalledWith(expect.anything(), '', historyUrl);
531+
expect(history.pushState).not.toHaveBeenCalled();
532+
});
533+
534+
it('pushes history when a POST changes the page URL', async () => {
535+
stubClientGlobals();
536+
window.location.pathname = '/books';
537+
window.location.search = '';
538+
vi.stubGlobal(
539+
'fetch',
540+
vi.fn(async () =>
541+
pageJson({ component: 'home', props: {}, url: '/books?page=2', errors: null }),
542+
),
543+
);
544+
const { router } = await createTestRouter();
545+
546+
await router.visit('/books?delete', { method: 'POST' });
547+
548+
expect(history.pushState).toHaveBeenCalledWith(expect.anything(), '', '/books?page=2');
549+
expect(history.replaceState).not.toHaveBeenCalled();
550+
});
551+
552+
it('does not grow history on repeated same-page named action submits', async () => {
553+
stubClientGlobals();
554+
window.location.pathname = '/db';
555+
window.location.search = '';
556+
vi.stubGlobal(
557+
'fetch',
558+
vi.fn(async () => pageJson({ component: 'home', props: {}, url: '/db', errors: null })),
559+
);
560+
const { router } = await createTestRouter();
561+
562+
await router.visit('/db?create', { method: 'POST' });
563+
await router.visit('/db?create', { method: 'POST' });
564+
565+
expect(history.pushState).not.toHaveBeenCalled();
566+
expect(history.replaceState).toHaveBeenCalledTimes(2);
567+
expect(history.replaceState).toHaveBeenLastCalledWith(expect.anything(), '', '/db');
528568
});
529569

530570
it('runs navigation listeners for same-page hash visits', async () => {

0 commit comments

Comments
 (0)