Skip to content

feat: use devalue for pushState/replaceState #14129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/four-pens-cheer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@sveltejs/kit": minor
---

feat: use the `transport` hook and `devalue` to serialize state in `pushState`/`replaceState`
50 changes: 19 additions & 31 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,13 @@ import {
} from './constants.js';
import { validate_page_exports } from '../../utils/exports.js';
import { compact } from '../../utils/array.js';
import { INVALIDATED_PARAM, TRAILING_SLASH_PARAM, validate_depends } from '../shared.js';
import {
INVALIDATED_PARAM,
stringify,
TRAILING_SLASH_PARAM,
validate_depends,
parse as unstringify
} from '../shared.js';
import { get_message, get_status } from '../../utils/error.js';
import { writable } from 'svelte/store';
import { page, update, navigating } from './state.svelte.js';
Expand Down Expand Up @@ -1590,7 +1596,7 @@ async function navigate({
const entry = {
[HISTORY_INDEX]: (current_history_index += change),
[NAVIGATION_INDEX]: (current_navigation_index += change),
[STATES_KEY]: state
[STATES_KEY]: stringify(state, app.hooks.transport)
};

const fn = replace_state ? history.replaceState : history.pushState;
Expand Down Expand Up @@ -2144,18 +2150,8 @@ export function pushState(url, state) {
throw new Error('Cannot call pushState(...) on the server');
}

if (DEV) {
if (!started) {
throw new Error('Cannot call pushState(...) before router is initialized');
}

try {
// use `devalue.stringify` as a convenient way to ensure we exclude values that can't be properly rehydrated, such as custom class instances
devalue.stringify(state);
} catch (error) {
// @ts-expect-error
throw new Error(`Could not serialize state${error.path}`);
}
if (DEV && !started) {
throw new Error('Cannot call pushState(...) before router is initialized');
}

update_scroll_positions(current_history_index);
Expand All @@ -2164,13 +2160,13 @@ export function pushState(url, state) {
[HISTORY_INDEX]: (current_history_index += 1),
[NAVIGATION_INDEX]: current_navigation_index,
[PAGE_URL_KEY]: page.url.href,
[STATES_KEY]: state
[STATES_KEY]: stringify(state, app.hooks.transport)
};

history.pushState(opts, '', resolve_url(url));
has_navigated = true;

page.state = state;
page.state = unstringify(opts[STATES_KEY], app.hooks.transport);
root.$set({
// we need to assign a new page object so that subscribers are correctly notified
page: untrack(() => clone_page(page))
Expand All @@ -2191,30 +2187,20 @@ export function replaceState(url, state) {
throw new Error('Cannot call replaceState(...) on the server');
}

if (DEV) {
if (!started) {
throw new Error('Cannot call replaceState(...) before router is initialized');
}

try {
// use `devalue.stringify` as a convenient way to ensure we exclude values that can't be properly rehydrated, such as custom class instances
devalue.stringify(state);
} catch (error) {
// @ts-expect-error
throw new Error(`Could not serialize state${error.path}`);
}
if (DEV && !started) {
throw new Error('Cannot call replaceState(...) before router is initialized');
}

const opts = {
[HISTORY_INDEX]: current_history_index,
[NAVIGATION_INDEX]: current_navigation_index,
[PAGE_URL_KEY]: page.url.href,
[STATES_KEY]: state
[STATES_KEY]: stringify(state, app.hooks.transport)
};

history.replaceState(opts, '', resolve_url(url));

page.state = state;
page.state = unstringify(opts[STATES_KEY], app.hooks.transport);
root.$set({
page: untrack(() => clone_page(page))
});
Expand Down Expand Up @@ -2519,7 +2505,9 @@ function _start_router() {
if (history_index === current_history_index) return;

const scroll = scroll_positions[history_index];
const state = event.state[STATES_KEY] ?? {};
const state = event.state[STATES_KEY]
? unstringify(event.state[STATES_KEY], app.hooks.transport)
: {};
const url = new URL(event.state[PAGE_URL_KEY] ?? location.href);
const navigation_index = event.state[NAVIGATION_INDEX];
const is_hash_change = current.url ? strip_hash(location) === strip_hash(current.url) : false;
Expand Down
15 changes: 12 additions & 3 deletions packages/kit/src/runtime/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,17 @@ export function stringify_remote_arg(value, transport) {
return base64_encode(bytes).replaceAll('=', '').replaceAll('+', '-').replaceAll('/', '_');
}

/**
* Parses `string` with `devalue.parse`, using the provided transport decoders.
* @param {string} string
* @param {Transport} transport
*/
export function parse(string, transport) {
const decoders = Object.fromEntries(Object.entries(transport).map(([k, v]) => [k, v.decode]));

return devalue.parse(string, decoders);
}

/**
* Parses the argument (if any) for a remote function
* @param {string} string
Expand All @@ -59,9 +70,7 @@ export function parse_remote_arg(string, transport) {
base64_decode(string.replaceAll('-', '+').replaceAll('_', '/'))
);

const decoders = Object.fromEntries(Object.entries(transport).map(([k, v]) => [k, v.decode]));

return devalue.parse(json_string, decoders);
return parse(json_string, transport);
}

/**
Expand Down
3 changes: 3 additions & 0 deletions packages/kit/test/apps/basics/src/app.d.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import type { Foo } from '$lib';

declare global {
namespace App {
interface Locals {
Expand All @@ -12,6 +14,7 @@ declare global {
interface PageState {
active?: boolean;
count?: number;
foo?: Foo;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<script>
import { pushState } from '$app/navigation';
import { page } from '$app/state';
import { Foo } from '$lib';
</script>

<button
onclick={() => {
const state = $state({ foo: new Foo('it works?'), count: 0 });
pushState('', state);
}}>push state</button
>

<!-- Make sure that page.state.count loses its reactivity -->
<button
onclick={() => {
page.state.count++;
}}
>
bump count
</button>

<p data-testid="foo">foo: {page.state.foo?.bar() ?? 'nope'}</p>
<p data-testid="count">count: {page.state.count ?? 'nope'}</p>
18 changes: 18 additions & 0 deletions packages/kit/test/apps/basics/test/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1308,7 +1308,7 @@
expect(page.locator('p.loadingfail')).toBeHidden();
});

test('Catches fetch errors from server load functions (direct hit)', async ({ page }) => {

Check warning on line 1311 in packages/kit/test/apps/basics/test/client.test.js

View workflow job for this annotation

GitHub Actions / test-kit (18, ubuntu-latest, chromium)

flaky test: Catches fetch errors from server load functions (direct hit)

retries: 2
page.goto('/streaming/server-error');
await expect(page.locator('p.eager')).toHaveText('eager');
await expect(page.locator('p.fail')).toHaveText('fail');
Expand Down Expand Up @@ -1497,6 +1497,24 @@
await page.locator('button').click();
await expect(page.locator('p')).toHaveText('count: 1');
});

test('pushState properly serializes objects', async ({ page }) => {
await page.goto('/shallow-routing/push-state/foo');
await expect(page.locator('[data-testid=foo]')).toHaveText('foo: nope');
await expect(page.locator('[data-testid=count]')).toHaveText('count: nope');

await page.getByText('push state').click();
await expect(page.locator('[data-testid=foo]')).toHaveText('foo: it works?!');
await expect(page.locator('[data-testid=count]')).toHaveText('count: 0');

await page.getByText('bump count').click();
await expect(page.locator('[data-testid=foo]')).toHaveText('foo: it works?!');
await expect(page.locator('[data-testid=count]')).toHaveText('count: 0'); // Ensure count is not bumped

await page.goBack();
await expect(page.locator('[data-testid=foo]')).toHaveText('foo: nope');
await expect(page.locator('[data-testid=count]')).toHaveText('count: nope');
});
});

test.describe('reroute', () => {
Expand Down
Loading