Skip to content

Commit

Permalink
fix: URL instance methods work in load (#5183)
Browse files Browse the repository at this point in the history
* feat: Failing test

* fix: More surgical approach to replacing url.hash

* feat: Changeset

* create LoadURL class

* use LoadURL on the server as well

Co-authored-by: Rich Harris <hello@rich-harris.dev>
  • Loading branch information
1 parent f5b409f commit 79959b0
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 14 deletions.
5 changes: 5 additions & 0 deletions .changeset/popular-pets-obey.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[fix] URL instance methods now work in `load`
16 changes: 3 additions & 13 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { onMount, tick } from 'svelte';
import { writable } from 'svelte/store';
import { coalesce_to_error } from '../../utils/error.js';
import { normalize } from '../load.js';
import { normalize_path } from '../../utils/url.js';
import { LoadURL, normalize_path } from '../../utils/url.js';
import {
create_updated_store,
find_anchor,
Expand Down Expand Up @@ -527,6 +527,7 @@ export function create_client({ target, session, base, trailing_slash }) {
}

const session = $session;
const load_url = new LoadURL(url);

if (module.load) {
/** @type {import('types').LoadEvent} */
Expand All @@ -536,18 +537,7 @@ export function create_client({ target, session, base, trailing_slash }) {
props: props || {},
get url() {
node.uses.url = true;

return new Proxy(url, {
get: (target, property) => {
if (property === 'hash') {
throw new Error(
'url.hash is inaccessible from load. Consider accessing hash from the page store within the script tag of your component.'
);
}

return Reflect.get(target, property, target);
}
});
return load_url;
},
get session() {
node.uses.session = true;
Expand Down
3 changes: 2 additions & 1 deletion packages/kit/src/runtime/server/page/load_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { normalize } from '../../load.js';
import { respond } from '../index.js';
import { is_root_relative, resolve } from '../../../utils/url.js';
import { create_prerendering_url_proxy } from './utils.js';
import { LoadURL } from '../../../utils/url.js';
import { is_pojo, lowercase_keys, normalize_request_method } from '../utils.js';
import { coalesce_to_error } from '../../../utils/error.js';
import { domain_matches, path_matches } from './cookie.js';
Expand Down Expand Up @@ -84,7 +85,7 @@ export async function load_node({
} else if (module.load) {
/** @type {import('types').LoadEvent} */
const load_input = {
url: state.prerendering ? create_prerendering_url_proxy(event.url) : event.url,
url: state.prerendering ? create_prerendering_url_proxy(event.url) : new LoadURL(event.url),
params: event.params,
props: shadow.body || {},
routeId: event.routeId,
Expand Down
9 changes: 9 additions & 0 deletions packages/kit/src/utils/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,12 @@ export function normalize_path(path, trailing_slash) {

return path;
}

export class LoadURL extends URL {
/** @returns {string} */
get hash() {
throw new Error(
'url.hash is inaccessible from load. Consider accessing hash from the page store within the script tag of your component.'
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script context="module">
export const load = ({ url }) => {
url.toString();
return {};
};
</script>

<h1>I didn't break!</h1>
7 changes: 7 additions & 0 deletions packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1516,6 +1516,13 @@ test.describe.parallel('Load', () => {
}
});

test('url instance methods work in load', async ({ page, javaScriptEnabled }) => {
if (javaScriptEnabled) {
await page.goto('/load/url-to-string');
expect(await page.textContent('h1')).toBe("I didn't break!");
}
});

test('using window.fetch causes a warning', async ({ page, javaScriptEnabled }) => {
if (javaScriptEnabled && process.env.DEV) {
const warnings = [];
Expand Down

0 comments on commit 79959b0

Please sign in to comment.