From 5d02a95c77473eb3750dcc44b713a4c6015702b0 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Sun, 22 Dec 2024 13:50:22 +0100 Subject: [PATCH] fix: do not mutate URL during reroute logic (#13222) fixes #13220 --- .changeset/rude-plants-rest.md | 5 +++++ packages/kit/src/exports/public.d.ts | 1 + packages/kit/src/runtime/client/client.js | 8 +++++--- .../apps/basics/src/routes/reroute/basic/b/+page.svelte | 6 +++++- packages/kit/test/apps/basics/test/client.test.js | 4 +++- packages/kit/test/apps/basics/test/server.test.js | 4 +++- packages/kit/test/apps/hash-based-routing/test/test.js | 4 ++-- packages/kit/types/index.d.ts | 1 + 8 files changed, 25 insertions(+), 8 deletions(-) create mode 100644 .changeset/rude-plants-rest.md diff --git a/.changeset/rude-plants-rest.md b/.changeset/rude-plants-rest.md new file mode 100644 index 000000000000..1cb48536c135 --- /dev/null +++ b/.changeset/rude-plants-rest.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: do not mutate URL during reroute logic diff --git a/packages/kit/src/exports/public.d.ts b/packages/kit/src/exports/public.d.ts index e9601bf4adc4..6bce6ce2aa50 100644 --- a/packages/kit/src/exports/public.d.ts +++ b/packages/kit/src/exports/public.d.ts @@ -622,6 +622,7 @@ export interface KitConfig { * What type of client-side router to use. * - `'pathname'` is the default and means the current URL pathname determines the route * - `'hash'` means the route is determined by `location.hash`. In this case, SSR and prerendering are disabled. This is only recommended if `pathname` is not an option, for example because you don't control the webserver where your app is deployed. + * It comes with some caveats: you can't use server-side rendering (or indeed any server logic), and you have to make sure that the links in your app all start with /#/, or they won't work. Beyond that, everything works exactly like a normal SvelteKit app. * * @default "pathname" * @since 2.14.0 diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 1b3c11161036..9f3451a68d51 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -1191,13 +1191,15 @@ function get_navigation_intent(url, invalidating) { rerouted = app.hooks.reroute({ url: new URL(url) }) ?? url; if (typeof rerouted === 'string') { + const tmp = new URL(url); // do not mutate the incoming URL + if (app.hash) { - url.hash = rerouted; + tmp.hash = rerouted; } else { - url.pathname = rerouted; + tmp.pathname = rerouted; } - rerouted = url; + rerouted = tmp; } } catch (e) { if (DEV) { diff --git a/packages/kit/test/apps/basics/src/routes/reroute/basic/b/+page.svelte b/packages/kit/test/apps/basics/src/routes/reroute/basic/b/+page.svelte index 540f76eae7b6..e405729c9633 100644 --- a/packages/kit/test/apps/basics/src/routes/reroute/basic/b/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/reroute/basic/b/+page.svelte @@ -1 +1,5 @@ -

Successfully rewritten

+ + +

Successfully rewritten, URL should still show a: {page.url.pathname}

diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js index d527a0e77870..313d2d5cb75d 100644 --- a/packages/kit/test/apps/basics/test/client.test.js +++ b/packages/kit/test/apps/basics/test/client.test.js @@ -1184,7 +1184,9 @@ test.describe('reroute', () => { test('Apply reroute during client side navigation', async ({ page }) => { await page.goto('/reroute/basic'); await page.click("a[href='/reroute/basic/a']"); - expect(await page.textContent('h1')).toContain('Successfully rewritten'); + expect(await page.textContent('h1')).toContain( + 'Successfully rewritten, URL should still show a: /reroute/basic/a' + ); }); test('Apply reroute after client-only redirects', async ({ page }) => { diff --git a/packages/kit/test/apps/basics/test/server.test.js b/packages/kit/test/apps/basics/test/server.test.js index 28fdc180b3c1..266373d5069d 100644 --- a/packages/kit/test/apps/basics/test/server.test.js +++ b/packages/kit/test/apps/basics/test/server.test.js @@ -642,7 +642,9 @@ test.describe('Miscellaneous', () => { test.describe('reroute', () => { test('Apply reroute when directly accessing a page', async ({ page }) => { await page.goto('/reroute/basic/a'); - expect(await page.textContent('h1')).toContain('Successfully rewritten'); + expect(await page.textContent('h1')).toContain( + 'Successfully rewritten, URL should still show a: /reroute/basic/a' + ); }); test('Returns a 500 response if reroute throws an error on the server', async ({ page }) => { diff --git a/packages/kit/test/apps/hash-based-routing/test/test.js b/packages/kit/test/apps/hash-based-routing/test/test.js index 52540e4a8f91..ae379c55f601 100644 --- a/packages/kit/test/apps/hash-based-routing/test/test.js +++ b/packages/kit/test/apps/hash-based-routing/test/test.js @@ -72,13 +72,13 @@ test.describe('hash based navigation', () => { await page.locator('a[href="/#/reroute-a"]').click(); await expect(page.locator('p')).toHaveText('rerouted'); let url = new URL(page.url()); - expect(url.hash).toBe('#/rerouted'); + expect(url.hash).toBe('#/reroute-a'); await page.goto('/'); await page.locator('a[href="/#/reroute-b"]').click(); await expect(page.locator('p')).toHaveText('rerouted'); url = new URL(page.url()); - expect(url.hash).toBe('#/rerouted'); + expect(url.hash).toBe('#/reroute-b'); }); }); diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index aa1b04ac9a7f..bd268a10ff31 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -604,6 +604,7 @@ declare module '@sveltejs/kit' { * What type of client-side router to use. * - `'pathname'` is the default and means the current URL pathname determines the route * - `'hash'` means the route is determined by `location.hash`. In this case, SSR and prerendering are disabled. This is only recommended if `pathname` is not an option, for example because you don't control the webserver where your app is deployed. + * It comes with some caveats: you can't use server-side rendering (or indeed any server logic), and you have to make sure that the links in your app all start with /#/, or they won't work. Beyond that, everything works exactly like a normal SvelteKit app. * * @default "pathname" * @since 2.14.0