From 8468af597c6240f7a3687ef1ed3873990b944f8c Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Tue, 9 Jan 2024 07:27:08 -0800 Subject: [PATCH] chore: shrink error messages shipped to client (#11551) * chore: shrink error messages shipped to client * format * skip unreachable code * remove outdated test * cast instead of return * run test only in dev mode --- .changeset/green-shrimps-carry.md | 5 +++++ packages/kit/src/runtime/client/client.js | 6 +++--- packages/kit/src/utils/routing.js | 9 +++------ packages/kit/src/utils/routing.spec.js | 5 ----- packages/kit/src/utils/url.js | 8 +++++--- packages/kit/test/apps/basics/test/client.test.js | 14 ++++++++------ 6 files changed, 24 insertions(+), 23 deletions(-) create mode 100644 .changeset/green-shrimps-carry.md diff --git a/.changeset/green-shrimps-carry.md b/.changeset/green-shrimps-carry.md new file mode 100644 index 000000000000..fc169f974c2f --- /dev/null +++ b/.changeset/green-shrimps-carry.md @@ -0,0 +1,5 @@ +--- +"@sveltejs/kit": patch +--- + +chore: shrink error messages shipped to client diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index f71e93d6f149..5bd4bd7bbc89 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -1121,7 +1121,7 @@ function _before_navigate({ url, type, intent, delta }) { ...nav.navigation, cancel: () => { should_block = true; - nav.reject(new Error('navigation was cancelled')); + nav.reject(new Error('navigation cancelled')); } }; @@ -1210,7 +1210,7 @@ async function navigate({ // abort if user navigated during update if (token !== nav_token) { - nav.reject(new Error('navigation was aborted')); + nav.reject(new Error('navigation aborted')); return false; } @@ -1903,7 +1903,7 @@ function _start_router() { ...nav.navigation, cancel: () => { should_block = true; - nav.reject(new Error('navigation was cancelled')); + nav.reject(new Error('navigation cancelled')); } }; diff --git a/packages/kit/src/utils/routing.js b/packages/kit/src/utils/routing.js index 66a70c5b3469..df31d9dec71c 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -62,12 +62,9 @@ export function parse_route_id(id) { ); } - const match = param_pattern.exec(content); - if (!match) { - throw new Error( - `Invalid param: ${content}. Params and matcher names can only have underscores and alphanumeric characters.` - ); - } + // We know the match cannot be null because manifest generation would have + // if we hit an invalid param/matcher name with non-alphanumeric character. + const match = /** @type {RegExpExecArray} */ (param_pattern.exec(content)); const [, is_optional, is_rest, name, matcher] = match; // It's assumed that the following invalid route id cases are already checked diff --git a/packages/kit/src/utils/routing.spec.js b/packages/kit/src/utils/routing.spec.js index e5595a28a4fb..de7fef129daa 100644 --- a/packages/kit/src/utils/routing.spec.js +++ b/packages/kit/src/utils/routing.spec.js @@ -61,11 +61,6 @@ describe('parse_route_id', () => { expect(actual.params).toEqual(expected.params); }); } - - test('errors on bad param name', () => { - assert.throws(() => parse_route_id('abc/[b-c]'), /Invalid param: b-c/); - assert.throws(() => parse_route_id('abc/[bc=d-e]'), /Invalid param: bc=d-e/); - }); }); describe('exec', () => { diff --git a/packages/kit/src/utils/url.js b/packages/kit/src/utils/url.js index 9f6391e42486..53759b31bbbd 100644 --- a/packages/kit/src/utils/url.js +++ b/packages/kit/src/utils/url.js @@ -1,4 +1,4 @@ -import { BROWSER } from 'esm-env'; +import { BROWSER, DEV } from 'esm-env'; /** * Matches a URI scheme. See https://www.rfc-editor.org/rfc/rfc3986#section-3.1 @@ -146,7 +146,9 @@ export function make_trackable(url, callback, search_params_callback) { }; } - disable_hash(tracked); + if (DEV || !BROWSER) { + disable_hash(tracked); + } return tracked; } @@ -155,7 +157,7 @@ export function make_trackable(url, callback, search_params_callback) { * Disallow access to `url.hash` on the server and in `load` * @param {URL} url */ -export function disable_hash(url) { +function disable_hash(url) { allow_nodejs_console_log(url); Object.defineProperty(url, 'hash', { diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js index fdf76661f67e..73c4477e709e 100644 --- a/packages/kit/test/apps/basics/test/client.test.js +++ b/packages/kit/test/apps/basics/test/client.test.js @@ -80,12 +80,14 @@ test.describe('Load', () => { expect(await page.textContent('h2')).toBe('x: b: 4'); }); - test('accessing url.hash from load errors and suggests using page store', async ({ page }) => { - await page.goto('/load/url-hash#please-dont-send-me-to-load'); - expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "Cannot access event.url.hash. Consider using `$page.url.hash` inside a component instead (500 Internal Error)"' - ); - }); + if (process.env.DEV) { + test('accessing url.hash from load errors and suggests using page store', async ({ page }) => { + await page.goto('/load/url-hash#please-dont-send-me-to-load'); + expect(await page.textContent('#message')).toBe( + 'This is your custom error page saying: "Cannot access event.url.hash. Consider using `$page.url.hash` inside a component instead (500 Internal Error)"' + ); + }); + } test('url instance methods work in load', async ({ page }) => { await page.goto('/load/url-to-string');