From 6b7d066d0a2e175dea4874e07e686d994d9e32e0 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 6 Dec 2023 11:10:24 +0100 Subject: [PATCH 01/14] breaking: disallow external navigation using `goto` by default closes #8775 --- .changeset/silent-games-taste.md | 5 +++++ packages/kit/src/runtime/app/navigation.js | 4 +++- packages/kit/src/runtime/client/client.js | 13 +++++++++++-- 3 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 .changeset/silent-games-taste.md diff --git a/.changeset/silent-games-taste.md b/.changeset/silent-games-taste.md new file mode 100644 index 000000000000..5783bf2ba088 --- /dev/null +++ b/.changeset/silent-games-taste.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': major +--- + +breaking: disallow external navigation using `goto` by default diff --git a/packages/kit/src/runtime/app/navigation.js b/packages/kit/src/runtime/app/navigation.js index 527a85fa14d7..ee70a7d45cc8 100644 --- a/packages/kit/src/runtime/app/navigation.js +++ b/packages/kit/src/runtime/app/navigation.js @@ -16,6 +16,7 @@ export const disableScrollHandling = /* @__PURE__ */ client_method('disable_scro * noScroll?: boolean; * keepFocus?: boolean; * invalidateAll?: boolean; + * external?: boolean; * state?: any * }) => Promise} * @param {string | URL} url Where to navigate to. Note that if you've set [`config.kit.paths.base`](https://kit.svelte.dev/docs/configuration#paths) and the URL is root-relative, you need to prepend the base path if you want to navigate within the app. @@ -23,7 +24,8 @@ export const disableScrollHandling = /* @__PURE__ */ client_method('disable_scro * @param {boolean} [opts.replaceState] If `true`, will replace the current `history` entry rather than creating a new one with `pushState` * @param {boolean} [opts.noScroll] If `true`, the browser will maintain its scroll position rather than scrolling to the top of the page after navigation * @param {boolean} [opts.keepFocus] If `true`, the currently focused element will retain focus after navigation. Otherwise, focus will be reset to the body - * @param {boolean} [invalidateAll] If `true`, all `load` functions of the page will be rerun. See https://kit.svelte.dev/docs/load#rerunning-load-functions for more info on invalidation. + * @param {boolean} [opts.invalidateAll] If `true`, all `load` functions of the page will be rerun. See https://kit.svelte.dev/docs/load#rerunning-load-functions for more info on invalidation. + * @param {boolean} [opts.external] By default, `goto` will not navigate to external URLs for consistency and security reasons. Set this to `true` to allow navigating to external URLs. * @param {any} [opts.state] The state of the new/updated history entry * @returns {Promise} */ diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 08465d6b5342..f438f79a9477 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -219,7 +219,7 @@ export function create_client(app, target) { /** * @param {string | URL} url - * @param {{ noScroll?: boolean; replaceState?: boolean; keepFocus?: boolean; state?: any; invalidateAll?: boolean }} opts + * @param {{ noScroll?: boolean; replaceState?: boolean; keepFocus?: boolean; state?: any; invalidateAll?: boolean, external?: boolean }} opts * @param {number} redirect_count * @param {{}} [nav_token] */ @@ -230,7 +230,8 @@ export function create_client(app, target) { replaceState = false, keepFocus = false, state = {}, - invalidateAll = false + invalidateAll = false, + external = false }, redirect_count, nav_token @@ -238,6 +239,14 @@ export function create_client(app, target) { if (typeof url === 'string') { url = new URL(url, get_base_uri(document)); } + if (!external && url.origin !== origin) { + if (DEV) { + throw new Error( + 'Cannot navigate to an external URL using `goto` unless the `external` option is set' + ); + } + return; + } return navigate({ url, From d3b62011509d81d04d6d2eb3b2577590c4fec222 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 6 Dec 2023 11:29:49 +0100 Subject: [PATCH 02/14] do it here instead --- packages/kit/src/runtime/client/client.js | 26 +++++++++++++---------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index f438f79a9477..a71fecc51430 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -219,7 +219,7 @@ export function create_client(app, target) { /** * @param {string | URL} url - * @param {{ noScroll?: boolean; replaceState?: boolean; keepFocus?: boolean; state?: any; invalidateAll?: boolean, external?: boolean }} opts + * @param {{ noScroll?: boolean; replaceState?: boolean; keepFocus?: boolean; state?: any; invalidateAll?: boolean }} opts * @param {number} redirect_count * @param {{}} [nav_token] */ @@ -230,8 +230,7 @@ export function create_client(app, target) { replaceState = false, keepFocus = false, state = {}, - invalidateAll = false, - external = false + invalidateAll = false }, redirect_count, nav_token @@ -239,14 +238,6 @@ export function create_client(app, target) { if (typeof url === 'string') { url = new URL(url, get_base_uri(document)); } - if (!external && url.origin !== origin) { - if (DEV) { - throw new Error( - 'Cannot navigate to an external URL using `goto` unless the `external` option is set' - ); - } - return; - } return navigate({ url, @@ -1385,6 +1376,19 @@ export function create_client(app, target) { }, goto: (href, opts = {}) => { + if (typeof href === 'string') { + href = new URL(href, get_base_uri(document)); + } + if (!opts.external && href.origin !== origin) { + if (DEV) { + return Promise.reject( + 'Cannot navigate to an external URL using `goto` unless the `external` option is set' + ); + } else { + return Promise.reject(); + } + } + return goto(href, opts, 0); }, From 23ef6af6d4cd8bc8429f7118de51473cbe4b31ce Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 6 Dec 2023 11:44:51 +0100 Subject: [PATCH 03/14] adjust test --- .../kit/test/apps/basics/test/cross-platform/client.test.js | 4 ++-- packages/kit/test/utils.d.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index 598c6e4c356e..1b52aab21002 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -147,7 +147,7 @@ test.describe('Navigation lifecycle functions', () => { baseURL }) => { await page.goto('/navigation-lifecycle/before-navigate/prevent-navigation'); - await app.goto('https://google.de'); + await app.goto('https://google.de', { external: true }); expect(page.url()).toBe(baseURL + '/navigation-lifecycle/before-navigate/prevent-navigation'); expect(await page.innerHTML('pre')).toBe('1 true goto'); }); @@ -216,7 +216,7 @@ test.describe('Navigation lifecycle functions', () => { await page.goto('/navigation-lifecycle/before-navigate/prevent-navigation'); await page.click('h1'); // The browsers block attempts to prevent navigation on a frame that's never had a user gesture. - await app.goto('https://google.de'); + await app.goto('https://google.de', { external: true }); await app.goto('/navigation-lifecycle/before-navigate/prevent-navigation?x=1'); expect(await page.innerHTML('pre')).toBe('2 false goto'); diff --git a/packages/kit/test/utils.d.ts b/packages/kit/test/utils.d.ts index 6ba3c0b8d89f..5ffab4b42b43 100644 --- a/packages/kit/test/utils.d.ts +++ b/packages/kit/test/utils.d.ts @@ -13,7 +13,7 @@ export const test: TestType< PlaywrightTestArgs & PlaywrightTestOptions & { app: { - goto(url: string, opts?: { replaceState?: boolean }): Promise; + goto(url: string, opts?: { replaceState?: boolean; external?: boolean }): Promise; invalidate(url: string): Promise; beforeNavigate(url: URL): void | boolean; afterNavigate(url: URL): void; From cd73187d084e75099f9f68b9565c119c2a13fca0 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 8 Dec 2023 17:55:49 +0100 Subject: [PATCH 04/14] remove option, TODO tests --- packages/kit/src/runtime/app/navigation.js | 2 -- packages/kit/src/runtime/client/client.js | 2 +- .../test/apps/basics/test/cross-platform/client.test.js | 7 ++++--- packages/kit/test/utils.d.ts | 2 +- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/kit/src/runtime/app/navigation.js b/packages/kit/src/runtime/app/navigation.js index ee70a7d45cc8..d8b942c5e358 100644 --- a/packages/kit/src/runtime/app/navigation.js +++ b/packages/kit/src/runtime/app/navigation.js @@ -16,7 +16,6 @@ export const disableScrollHandling = /* @__PURE__ */ client_method('disable_scro * noScroll?: boolean; * keepFocus?: boolean; * invalidateAll?: boolean; - * external?: boolean; * state?: any * }) => Promise} * @param {string | URL} url Where to navigate to. Note that if you've set [`config.kit.paths.base`](https://kit.svelte.dev/docs/configuration#paths) and the URL is root-relative, you need to prepend the base path if you want to navigate within the app. @@ -25,7 +24,6 @@ export const disableScrollHandling = /* @__PURE__ */ client_method('disable_scro * @param {boolean} [opts.noScroll] If `true`, the browser will maintain its scroll position rather than scrolling to the top of the page after navigation * @param {boolean} [opts.keepFocus] If `true`, the currently focused element will retain focus after navigation. Otherwise, focus will be reset to the body * @param {boolean} [opts.invalidateAll] If `true`, all `load` functions of the page will be rerun. See https://kit.svelte.dev/docs/load#rerunning-load-functions for more info on invalidation. - * @param {boolean} [opts.external] By default, `goto` will not navigate to external URLs for consistency and security reasons. Set this to `true` to allow navigating to external URLs. * @param {any} [opts.state] The state of the new/updated history entry * @returns {Promise} */ diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index a71fecc51430..a6d8e35c9bb7 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -1379,7 +1379,7 @@ export function create_client(app, target) { if (typeof href === 'string') { href = new URL(href, get_base_uri(document)); } - if (!opts.external && href.origin !== origin) { + if (href.origin !== origin) { if (DEV) { return Promise.reject( 'Cannot navigate to an external URL using `goto` unless the `external` option is set' diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index 1b52aab21002..32fbd4bc1267 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -143,11 +143,11 @@ test.describe('Navigation lifecycle functions', () => { test('beforeNavigate prevents external navigation triggered by goto', async ({ page, - app, baseURL }) => { await page.goto('/navigation-lifecycle/before-navigate/prevent-navigation'); - await app.goto('https://google.de', { external: true }); + page.goto('https://google.de'); + await page.waitForTimeout(500); expect(page.url()).toBe(baseURL + '/navigation-lifecycle/before-navigate/prevent-navigation'); expect(await page.innerHTML('pre')).toBe('1 true goto'); }); @@ -216,7 +216,8 @@ test.describe('Navigation lifecycle functions', () => { await page.goto('/navigation-lifecycle/before-navigate/prevent-navigation'); await page.click('h1'); // The browsers block attempts to prevent navigation on a frame that's never had a user gesture. - await app.goto('https://google.de', { external: true }); + page.goto('https://google.de'); + await page.waitForTimeout(500); await app.goto('/navigation-lifecycle/before-navigate/prevent-navigation?x=1'); expect(await page.innerHTML('pre')).toBe('2 false goto'); diff --git a/packages/kit/test/utils.d.ts b/packages/kit/test/utils.d.ts index 5ffab4b42b43..6ba3c0b8d89f 100644 --- a/packages/kit/test/utils.d.ts +++ b/packages/kit/test/utils.d.ts @@ -13,7 +13,7 @@ export const test: TestType< PlaywrightTestArgs & PlaywrightTestOptions & { app: { - goto(url: string, opts?: { replaceState?: boolean; external?: boolean }): Promise; + goto(url: string, opts?: { replaceState?: boolean }): Promise; invalidate(url: string): Promise; beforeNavigate(url: URL): void | boolean; afterNavigate(url: URL): void; From 68e379971db5fd5fe6987b6ab2a2dd4199b2686a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 8 Dec 2023 12:50:57 -0500 Subject: [PATCH 05/14] tweak message --- packages/kit/src/runtime/client/client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index a6d8e35c9bb7..fc98126a523f 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -1382,7 +1382,7 @@ export function create_client(app, target) { if (href.origin !== origin) { if (DEV) { return Promise.reject( - 'Cannot navigate to an external URL using `goto` unless the `external` option is set' + `Cannot use \`goto\` with an external URL. Use \`window.location = "${href}"\` instead` ); } else { return Promise.reject(); From 11a498a82f837311d1fea38399fafabdcf568cca Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 8 Dec 2023 12:51:36 -0500 Subject: [PATCH 06/14] Update .changeset/silent-games-taste.md --- .changeset/silent-games-taste.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/silent-games-taste.md b/.changeset/silent-games-taste.md index 5783bf2ba088..fcd66101a965 100644 --- a/.changeset/silent-games-taste.md +++ b/.changeset/silent-games-taste.md @@ -2,4 +2,4 @@ '@sveltejs/kit': major --- -breaking: disallow external navigation using `goto` by default +breaking: disallow external navigation with `goto` From 6f83f04f834a1d2e75ab9cab7beaa8ad0b0dfe57 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 8 Dec 2023 13:17:52 -0500 Subject: [PATCH 07/14] add a test, remove an obsolete test --- packages/kit/src/runtime/client/client.js | 22 +++++++++---------- .../apps/basics/src/routes/goto/+page.svelte | 15 +++++++++++++ .../kit/test/apps/basics/test/client.test.js | 10 +++++++++ .../basics/test/cross-platform/client.test.js | 11 ---------- 4 files changed, 36 insertions(+), 22 deletions(-) create mode 100644 packages/kit/test/apps/basics/src/routes/goto/+page.svelte diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index fc98126a523f..68fc7a98f7be 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -546,10 +546,10 @@ export function create_client(app, target) { typeof data !== 'object' ? `a ${typeof data}` : data instanceof Response - ? 'a Response object' - : Array.isArray(data) - ? 'an array' - : 'a non-plain object' + ? 'a Response object' + : Array.isArray(data) + ? 'an array' + : 'a non-plain object' }, but must return a plain object at the top level (i.e. \`return {...}\`)` ); } @@ -1380,13 +1380,13 @@ export function create_client(app, target) { href = new URL(href, get_base_uri(document)); } if (href.origin !== origin) { - if (DEV) { - return Promise.reject( - `Cannot use \`goto\` with an external URL. Use \`window.location = "${href}"\` instead` - ); - } else { - return Promise.reject(); - } + return Promise.reject( + new Error( + DEV + ? `Cannot use \`goto\` with an external URL. Use \`window.location = "${href}"\` instead` + : 'Invalid URL' + ) + ); } return goto(href, opts, 0); diff --git a/packages/kit/test/apps/basics/src/routes/goto/+page.svelte b/packages/kit/test/apps/basics/src/routes/goto/+page.svelte new file mode 100644 index 000000000000..b5a261b83572 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/goto/+page.svelte @@ -0,0 +1,15 @@ + + + + +

{message}

diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js index aac4812f51c1..0ca6d56775bf 100644 --- a/packages/kit/test/apps/basics/test/client.test.js +++ b/packages/kit/test/apps/basics/test/client.test.js @@ -841,3 +841,13 @@ test.describe('Assets', () => { ).toBe(true); }); }); + +test.describe('goto', () => { + test('goto fails with external URL', async ({ page }) => { + await page.goto('/goto'); + await page.click('button'); + + const message = process.env.DEV ? 'Cannot use `goto` with an external URL. Use `window.location = \"https://example.com/\"` instead' : 'Invalid URL'; + await expect(page.locator('p')).toHaveText(message); + }); +}); diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index 32fbd4bc1267..1e5e08fbb02e 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -141,17 +141,6 @@ test.describe('Navigation lifecycle functions', () => { expect(await page.innerHTML('pre')).toBe('1 false goto'); }); - test('beforeNavigate prevents external navigation triggered by goto', async ({ - page, - baseURL - }) => { - await page.goto('/navigation-lifecycle/before-navigate/prevent-navigation'); - page.goto('https://google.de'); - await page.waitForTimeout(500); - expect(page.url()).toBe(baseURL + '/navigation-lifecycle/before-navigate/prevent-navigation'); - expect(await page.innerHTML('pre')).toBe('1 true goto'); - }); - test('beforeNavigate prevents navigation triggered by back button', async ({ page, app, From 9d3a5c5a24908c10033f7b3fa6fa4d0ca7504ede Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 8 Dec 2023 13:42:17 -0500 Subject: [PATCH 08/14] fix test --- .../kit/test/apps/basics/test/cross-platform/client.test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index 1e5e08fbb02e..0c6b02f324ac 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -205,8 +205,7 @@ test.describe('Navigation lifecycle functions', () => { await page.goto('/navigation-lifecycle/before-navigate/prevent-navigation'); await page.click('h1'); // The browsers block attempts to prevent navigation on a frame that's never had a user gesture. - page.goto('https://google.de'); - await page.waitForTimeout(500); + await page.click('[href="https://google.de"]'); await app.goto('/navigation-lifecycle/before-navigate/prevent-navigation?x=1'); expect(await page.innerHTML('pre')).toBe('2 false goto'); From 83b69505104578519b00d9a8629db2007fc3ecc6 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 8 Dec 2023 13:45:45 -0500 Subject: [PATCH 09/14] prettier --- packages/kit/src/runtime/client/client.js | 8 ++++---- .../apps/basics/src/routes/goto/+page.svelte | 16 +++++++++------- .../kit/test/apps/basics/test/client.test.js | 4 +++- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 68fc7a98f7be..10bae4568a4f 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -546,10 +546,10 @@ export function create_client(app, target) { typeof data !== 'object' ? `a ${typeof data}` : data instanceof Response - ? 'a Response object' - : Array.isArray(data) - ? 'an array' - : 'a non-plain object' + ? 'a Response object' + : Array.isArray(data) + ? 'an array' + : 'a non-plain object' }, but must return a plain object at the top level (i.e. \`return {...}\`)` ); } diff --git a/packages/kit/test/apps/basics/src/routes/goto/+page.svelte b/packages/kit/test/apps/basics/src/routes/goto/+page.svelte index b5a261b83572..a3920c936119 100644 --- a/packages/kit/test/apps/basics/src/routes/goto/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/goto/+page.svelte @@ -4,12 +4,14 @@ let message = '...'; - +

{message}

diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js index 0ca6d56775bf..538b23ba4be8 100644 --- a/packages/kit/test/apps/basics/test/client.test.js +++ b/packages/kit/test/apps/basics/test/client.test.js @@ -847,7 +847,9 @@ test.describe('goto', () => { await page.goto('/goto'); await page.click('button'); - const message = process.env.DEV ? 'Cannot use `goto` with an external URL. Use `window.location = \"https://example.com/\"` instead' : 'Invalid URL'; + const message = process.env.DEV + ? 'Cannot use `goto` with an external URL. Use `window.location = "https://example.com/"` instead' + : 'Invalid URL'; await expect(page.locator('p')).toHaveText(message); }); }); From 581c8a73675ace45072a635846447d9e84c1989b Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 8 Dec 2023 19:59:17 +0100 Subject: [PATCH 10/14] fix --- packages/kit/src/exports/public.d.ts | 2 +- .../test/apps/basics/test/cross-platform/client.test.js | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/kit/src/exports/public.d.ts b/packages/kit/src/exports/public.d.ts index 05fa8d832e2d..b2e7cc072384 100644 --- a/packages/kit/src/exports/public.d.ts +++ b/packages/kit/src/exports/public.d.ts @@ -846,7 +846,7 @@ export interface Navigation { /** * The type of navigation: * - `form`: The user submitted a `
` - * - `leave`: The user is leaving the app by closing the tab or using the back/forward buttons to go to a different document + * - `leave`: The user is leaving the app by closing the tab or using the back/forward buttons to go to a different document, or as a result of programmatic external navigation such as `location.href = ...` * - `link`: Navigation was triggered by a link click * - `goto`: Navigation was triggered by a `goto(...)` call or a redirect * - `popstate`: Navigation was triggered by back/forward navigation diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index 0c6b02f324ac..7392c2465c41 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -204,8 +204,11 @@ test.describe('Navigation lifecycle functions', () => { }) => { await page.goto('/navigation-lifecycle/before-navigate/prevent-navigation'); await page.click('h1'); // The browsers block attempts to prevent navigation on a frame that's never had a user gesture. - - await page.click('[href="https://google.de"]'); + page.on('dialog', async (dialog) => { + await dialog.dismiss(); + }); + await page.close({ runBeforeUnload: true }); + await page.waitForTimeout(100); await app.goto('/navigation-lifecycle/before-navigate/prevent-navigation?x=1'); expect(await page.innerHTML('pre')).toBe('2 false goto'); From 4ddf16d6b31b67c1f4329f53ccc3e773a6aec837 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 8 Dec 2023 17:35:24 -0500 Subject: [PATCH 11/14] Update packages/kit/src/exports/public.d.ts Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> --- packages/kit/src/exports/public.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/exports/public.d.ts b/packages/kit/src/exports/public.d.ts index b2e7cc072384..31ac6094be04 100644 --- a/packages/kit/src/exports/public.d.ts +++ b/packages/kit/src/exports/public.d.ts @@ -846,7 +846,7 @@ export interface Navigation { /** * The type of navigation: * - `form`: The user submitted a `` - * - `leave`: The user is leaving the app by closing the tab or using the back/forward buttons to go to a different document, or as a result of programmatic external navigation such as `location.href = ...` + * - `leave`: The app is being left either because the tab is being closed or a navigation to a different document is occurring * - `link`: Navigation was triggered by a link click * - `goto`: Navigation was triggered by a `goto(...)` call or a redirect * - `popstate`: Navigation was triggered by back/forward navigation From b8fe747d29407f93201178061ba40fc9ef97de20 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 8 Dec 2023 17:49:55 -0500 Subject: [PATCH 12/14] Update packages/kit/src/runtime/client/client.js Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> --- packages/kit/src/runtime/client/client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 10bae4568a4f..3606a342c14f 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -1384,7 +1384,7 @@ export function create_client(app, target) { new Error( DEV ? `Cannot use \`goto\` with an external URL. Use \`window.location = "${href}"\` instead` - : 'Invalid URL' + : 'goto: invalid URL' ) ); } From 830f0e2fa4513857ad6be00fbb8d422953814726 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 8 Dec 2023 19:29:38 -0500 Subject: [PATCH 13/14] fix test --- packages/kit/test/apps/basics/test/client.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js index 538b23ba4be8..26c7df9d2be3 100644 --- a/packages/kit/test/apps/basics/test/client.test.js +++ b/packages/kit/test/apps/basics/test/client.test.js @@ -849,7 +849,7 @@ test.describe('goto', () => { const message = process.env.DEV ? 'Cannot use `goto` with an external URL. Use `window.location = "https://example.com/"` instead' - : 'Invalid URL'; + : 'goto: invalid URL'; await expect(page.locator('p')).toHaveText(message); }); }); From 87dc49fd8e8a3e67f9e3126cab74a1c7fa125da8 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 8 Dec 2023 19:55:44 -0500 Subject: [PATCH 14/14] DRY out --- packages/kit/src/runtime/client/client.js | 23 +++++++++-------------- packages/kit/src/runtime/client/utils.js | 14 ++++++++------ 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 3606a342c14f..d5e66430424d 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -18,7 +18,7 @@ import { parse } from './parse.js'; import * as storage from './session-storage.js'; import { find_anchor, - get_base_uri, + resolve_url, get_link_info, get_router_options, is_external_url, @@ -235,12 +235,8 @@ export function create_client(app, target) { redirect_count, nav_token ) { - if (typeof url === 'string') { - url = new URL(url, get_base_uri(document)); - } - return navigate({ - url, + url: resolve_url(url), scroll: noScroll ? scroll_state() : null, keepfocus: keepFocus, redirect_count, @@ -1375,21 +1371,20 @@ export function create_client(app, target) { } }, - goto: (href, opts = {}) => { - if (typeof href === 'string') { - href = new URL(href, get_base_uri(document)); - } - if (href.origin !== origin) { + goto: (url, opts = {}) => { + url = resolve_url(url); + + if (url.origin !== origin) { return Promise.reject( new Error( DEV - ? `Cannot use \`goto\` with an external URL. Use \`window.location = "${href}"\` instead` + ? `Cannot use \`goto\` with an external URL. Use \`window.location = "${url}"\` instead` : 'goto: invalid URL' ) ); } - return goto(href, opts, 0); + return goto(url, opts, 0); }, invalidate: (resource) => { @@ -1409,7 +1404,7 @@ export function create_client(app, target) { }, preload_data: async (href) => { - const url = new URL(href, get_base_uri(document)); + const url = resolve_url(href); const intent = get_navigation_intent(url, false); if (!intent) { diff --git a/packages/kit/src/runtime/client/utils.js b/packages/kit/src/runtime/client/utils.js index d330ff8c0c64..b9cbc8c62c89 100644 --- a/packages/kit/src/runtime/client/utils.js +++ b/packages/kit/src/runtime/client/utils.js @@ -8,16 +8,18 @@ import { PRELOAD_PRIORITIES } from './constants.js'; export const origin = BROWSER ? location.origin : ''; -/** @param {HTMLDocument} doc */ -export function get_base_uri(doc) { - let baseURI = doc.baseURI; +/** @param {string | URL} url */ +export function resolve_url(url) { + if (url instanceof URL) return url; + + let baseURI = document.baseURI; if (!baseURI) { - const baseTags = doc.getElementsByTagName('base'); - baseURI = baseTags.length ? baseTags[0].href : doc.URL; + const baseTags = document.getElementsByTagName('base'); + baseURI = baseTags.length ? baseTags[0].href : document.URL; } - return baseURI; + return new URL(url, baseURI); } export function scroll_state() {