Skip to content

Commit

Permalink
fix: do not mutate URL during reroute logic (#13222)
Browse files Browse the repository at this point in the history
fixes #13220
  • Loading branch information
dummdidumm authored Dec 22, 2024
1 parent e95d446 commit 5d02a95
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 8 deletions.
5 changes: 5 additions & 0 deletions .changeset/rude-plants-rest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: do not mutate URL during reroute logic
1 change: 1 addition & 0 deletions packages/kit/src/exports/public.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
<h1>Successfully rewritten</h1>
<script>
import { page } from '$app/state';
</script>

<h1>Successfully rewritten, URL should still show a: {page.url.pathname}</h1>
4 changes: 3 additions & 1 deletion packages/kit/test/apps/basics/test/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => {
Expand Down
4 changes: 3 additions & 1 deletion packages/kit/test/apps/basics/test/server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => {
Expand Down
4 changes: 2 additions & 2 deletions packages/kit/test/apps/hash-based-routing/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
1 change: 1 addition & 0 deletions packages/kit/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 5d02a95

Please sign in to comment.