From 7c3703ebf69bc8db3c6b242fda068cdfac9c024d Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Sun, 3 Dec 2023 09:57:42 -0800 Subject: [PATCH 1/9] breaking: undefined is no longer a valid value for paths.relative --- .changeset/tall-suns-protect.md | 5 +++++ packages/kit/src/core/config/index.spec.js | 8 ++++---- packages/kit/src/core/config/options.js | 4 ++-- packages/kit/src/exports/public.d.ts | 4 ++-- packages/kit/src/runtime/server/page/render.js | 2 +- 5 files changed, 14 insertions(+), 9 deletions(-) create mode 100644 .changeset/tall-suns-protect.md diff --git a/.changeset/tall-suns-protect.md b/.changeset/tall-suns-protect.md new file mode 100644 index 000000000000..21f8fa055fa0 --- /dev/null +++ b/.changeset/tall-suns-protect.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': major +--- + +breaking: undefined is no longer a valid value for paths.relative diff --git a/packages/kit/src/core/config/index.spec.js b/packages/kit/src/core/config/index.spec.js index faaf138aebb3..e0e29a009ae5 100644 --- a/packages/kit/src/core/config/index.spec.js +++ b/packages/kit/src/core/config/index.spec.js @@ -102,7 +102,7 @@ const get_defaults = (prefix = '') => ({ paths: { base: '', assets: '', - relative: undefined + relative: true }, prerender: { concurrency: 1, @@ -321,7 +321,7 @@ validate_paths( { base: '/path/to/base', assets: '', - relative: undefined + relative: true } ); @@ -333,7 +333,7 @@ validate_paths( { base: '', assets: 'https://cdn.example.com', - relative: undefined + relative: true } ); @@ -346,7 +346,7 @@ validate_paths( { base: '/path/to/base', assets: 'https://cdn.example.com', - relative: undefined + relative: true } ); diff --git a/packages/kit/src/core/config/options.js b/packages/kit/src/core/config/options.js index 2d0c928d9290..1dc79d5b1eb7 100644 --- a/packages/kit/src/core/config/options.js +++ b/packages/kit/src/core/config/options.js @@ -179,9 +179,9 @@ const options = object( return input; }), - relative: validate(undefined, (input, keypath) => { + relative: validate(true, (input, keypath) => { if (typeof input !== 'boolean') { - throw new Error(`${keypath} option must be a boolean or undefined`); + throw new Error(`${keypath} option must be a boolean`); } return input; diff --git a/packages/kit/src/exports/public.d.ts b/packages/kit/src/exports/public.d.ts index 77f15a391898..a25254c3c46f 100644 --- a/packages/kit/src/exports/public.d.ts +++ b/packages/kit/src/exports/public.d.ts @@ -473,7 +473,7 @@ export interface KitConfig { */ base?: '' | `/${string}`; /** - * Whether to use relative asset paths. By default, if `paths.assets` is not external, SvelteKit will replace `%sveltekit.assets%` with a relative path and use relative paths to reference build artifacts, but `base` and `assets` imported from `$app/paths` will be as specified in your config. + * Whether to use relative asset paths. `true` by default in order to support things like the Internet Archive. * * If `true`, `base` and `assets` imported from `$app/paths` will be replaced with relative asset paths during server-side rendering, resulting in portable HTML. * If `false`, `%sveltekit.assets%` and references to build artifacts will always be root-relative paths, unless `paths.assets` is an external URL @@ -481,7 +481,7 @@ export interface KitConfig { * If your app uses a `` element, you should set this to `false`, otherwise asset URLs will incorrectly be resolved against the `` URL rather than the current page. * @default undefined */ - relative?: boolean | undefined; + relative?: boolean; }; /** * See [Prerendering](https://kit.svelte.dev/docs/page-options#prerender). diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js index dbc558151fd2..0edbeb333dab 100644 --- a/packages/kit/src/runtime/server/page/render.js +++ b/packages/kit/src/runtime/server/page/render.js @@ -95,7 +95,7 @@ export async function render_response({ let base_expression = s(paths.base); // if appropriate, use relative paths for greater portability - if (paths.relative !== false && !state.prerendering?.fallback) { + if (paths.relative && !state.prerendering?.fallback) { const segments = event.url.pathname.slice(paths.base.length).split('/').slice(2); base = segments.map(() => '..').join('/') || '.'; From 6c60f9c4a62b578350585514461071074adfee87 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Mon, 4 Dec 2023 06:33:22 -0800 Subject: [PATCH 2/9] docs updates --- packages/kit/src/exports/public.d.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/kit/src/exports/public.d.ts b/packages/kit/src/exports/public.d.ts index a25254c3c46f..dc0ee49e49be 100644 --- a/packages/kit/src/exports/public.d.ts +++ b/packages/kit/src/exports/public.d.ts @@ -473,13 +473,16 @@ export interface KitConfig { */ base?: '' | `/${string}`; /** - * Whether to use relative asset paths. `true` by default in order to support things like the Internet Archive. + * Whether to use relative asset paths. `true` by default in order to support things like the Internet Archive. `false` may have slight performance advantages and may be preferrable for internal applications. * * If `true`, `base` and `assets` imported from `$app/paths` will be replaced with relative asset paths during server-side rendering, resulting in portable HTML. * If `false`, `%sveltekit.assets%` and references to build artifacts will always be root-relative paths, unless `paths.assets` is an external URL * * If your app uses a `` element, you should set this to `false`, otherwise asset URLs will incorrectly be resolved against the `` URL rather than the current page. - * @default undefined + * + * In 1.0, `undefined` was a valid value, which was set by default. In that case, if `paths.assets` was not external, SvelteKit would replace `%sveltekit.assets%` with a relative path and use relative paths to reference build artifacts, but `base` and `assets` imported from `$app/paths` would be as specified in your config. + * + * @default true */ relative?: boolean; }; From 6205e9a0e48fd16c2350255987d83a9bca4f868c Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Mon, 4 Dec 2023 09:21:17 -0800 Subject: [PATCH 3/9] update test --- packages/kit/test/apps/basics/test/test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index d4b56ba50c92..cc8e55f9b6dd 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -647,8 +647,8 @@ test.describe('$app/paths', () => { expect(await page.innerHTML('pre')).toBe( JSON.stringify({ - base: '', - assets: '' + base: '.', + assets: '.' }) ); @@ -656,8 +656,8 @@ test.describe('$app/paths', () => { expect(await page.innerHTML('pre')).toBe( JSON.stringify({ - base: '', - assets: '' + base: '.', + assets: '.' }) ); }); From a8ad756ecc994567fe04a638e06e88eb42a88582 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Mon, 4 Dec 2023 09:35:18 -0800 Subject: [PATCH 4/9] try updating test again --- packages/kit/test/apps/basics/test/test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index cc8e55f9b6dd..d0ac1be402f5 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -647,8 +647,8 @@ test.describe('$app/paths', () => { expect(await page.innerHTML('pre')).toBe( JSON.stringify({ - base: '.', - assets: '.' + base: '', + assets: '' }) ); @@ -656,8 +656,8 @@ test.describe('$app/paths', () => { expect(await page.innerHTML('pre')).toBe( JSON.stringify({ - base: '.', - assets: '.' + base: '../..', + assets: '../..' }) ); }); From ea337a28f4a88f8d79da0665bab87a1145c461e3 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Sat, 9 Dec 2023 20:57:37 -0800 Subject: [PATCH 5/9] make test pass. but is this right? --- packages/kit/test/apps/basics/test/test.js | 44 +++++++++++++++------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index d0ac1be402f5..e87dda8b1627 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -642,24 +642,42 @@ test.describe('$app/environment', () => { }); test.describe('$app/paths', () => { - test('includes paths', async ({ page }) => { + test('includes paths', async ({ page, javaScriptEnabled }) => { await page.goto('/paths'); - expect(await page.innerHTML('pre')).toBe( - JSON.stringify({ - base: '', - assets: '' - }) - ); + if (javaScriptEnabled) { + expect(await page.innerHTML('pre')).toBe( + JSON.stringify({ + base: '', + assets: '' + }) + ); + } else { + expect(await page.innerHTML('pre')).toBe( + JSON.stringify({ + base: '.', + assets: '.' + }) + ); + } await page.goto('/paths/deeply/nested'); - expect(await page.innerHTML('pre')).toBe( - JSON.stringify({ - base: '../..', - assets: '../..' - }) - ); + if (javaScriptEnabled) { + expect(await page.innerHTML('pre')).toBe( + JSON.stringify({ + base: '', + assets: '' + }) + ); + } else { + expect(await page.innerHTML('pre')).toBe( + JSON.stringify({ + base: '../..', + assets: '../..' + }) + ); + } }); // some browsers will re-request assets after a `pushState` From dfefaf27e073b5b3005660bdfd252a1787eaba48 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Sat, 9 Dec 2023 21:00:03 -0800 Subject: [PATCH 6/9] Update packages/kit/src/exports/public.d.ts Co-authored-by: Rich Harris --- 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 6a14d73916fe..81896d933884 100644 --- a/packages/kit/src/exports/public.d.ts +++ b/packages/kit/src/exports/public.d.ts @@ -473,7 +473,7 @@ export interface KitConfig { */ base?: '' | `/${string}`; /** - * Whether to use relative asset paths. `true` by default in order to support things like the Internet Archive. `false` may have slight performance advantages and may be preferrable for internal applications. + * Whether to use relative asset paths. * * If `true`, `base` and `assets` imported from `$app/paths` will be replaced with relative asset paths during server-side rendering, resulting in portable HTML. * If `false`, `%sveltekit.assets%` and references to build artifacts will always be root-relative paths, unless `paths.assets` is an external URL From b25a68aed7cb7f29524d0d2fe1ecd87a5e5c28dc Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Sat, 9 Dec 2023 21:00:18 -0800 Subject: [PATCH 7/9] Update packages/kit/src/exports/public.d.ts Co-authored-by: Rich Harris --- 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 81896d933884..37f394588435 100644 --- a/packages/kit/src/exports/public.d.ts +++ b/packages/kit/src/exports/public.d.ts @@ -475,7 +475,7 @@ export interface KitConfig { /** * Whether to use relative asset paths. * - * If `true`, `base` and `assets` imported from `$app/paths` will be replaced with relative asset paths during server-side rendering, resulting in portable HTML. + * If `true`, `base` and `assets` imported from `$app/paths` will be replaced with relative asset paths during server-side rendering, resulting in more portable HTML. * If `false`, `%sveltekit.assets%` and references to build artifacts will always be root-relative paths, unless `paths.assets` is an external URL * * If your app uses a `` element, you should set this to `false`, otherwise asset URLs will incorrectly be resolved against the `` URL rather than the current page. From 878b71b4f97fc2dd93e849594d3eccc24b1b0fee Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 10 Dec 2023 13:21:10 -0500 Subject: [PATCH 8/9] simplify --- packages/kit/src/core/config/options.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/kit/src/core/config/options.js b/packages/kit/src/core/config/options.js index 1dc79d5b1eb7..67f593e2d590 100644 --- a/packages/kit/src/core/config/options.js +++ b/packages/kit/src/core/config/options.js @@ -179,13 +179,7 @@ const options = object( return input; }), - relative: validate(true, (input, keypath) => { - if (typeof input !== 'boolean') { - throw new Error(`${keypath} option must be a boolean`); - } - - return input; - }) + relative: boolean(true) }), prerender: object({ From 118b9f1b678bbfe500525203fa42f74d7f455605 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 10 Dec 2023 13:22:33 -0500 Subject: [PATCH 9/9] simplify --- packages/kit/test/apps/basics/test/test.js | 34 +++------------------- 1 file changed, 4 insertions(+), 30 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index e87dda8b1627..b64daa01ea31 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -645,39 +645,13 @@ test.describe('$app/paths', () => { test('includes paths', async ({ page, javaScriptEnabled }) => { await page.goto('/paths'); - if (javaScriptEnabled) { - expect(await page.innerHTML('pre')).toBe( - JSON.stringify({ - base: '', - assets: '' - }) - ); - } else { - expect(await page.innerHTML('pre')).toBe( - JSON.stringify({ - base: '.', - assets: '.' - }) - ); - } + let base = javaScriptEnabled ? '' : '.'; + expect(await page.innerHTML('pre')).toBe(JSON.stringify({ base, assets: base })); await page.goto('/paths/deeply/nested'); - if (javaScriptEnabled) { - expect(await page.innerHTML('pre')).toBe( - JSON.stringify({ - base: '', - assets: '' - }) - ); - } else { - expect(await page.innerHTML('pre')).toBe( - JSON.stringify({ - base: '../..', - assets: '../..' - }) - ); - } + base = javaScriptEnabled ? '' : '../..'; + expect(await page.innerHTML('pre')).toBe(JSON.stringify({ base, assets: base })); }); // some browsers will re-request assets after a `pushState`