Skip to content

Commit

Permalink
[fix] Apply updated stuff (and other page properties) when `get_n…
Browse files Browse the repository at this point in the history
…avigation_result_from_branch` called (#4392)

* add test

* apply updated props.page when update / goto page

* add changeset

* fix lint

* fix formatting

* update page store more conservatively

Co-authored-by: Rich Harris <hello@rich-harris.dev>
  • Loading branch information
baseballyama and Rich-Harris authored Apr 15, 2022
1 parent f445802 commit e5f8065
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/serious-elephants-dress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

apply updated `props.page` when update or goto page
28 changes: 20 additions & 8 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ const routes = parse(components, dictionary, matchers);
const default_layout = components[0]();
const default_error = components[1]();

const root_stuff = {};

// We track the scroll position associated with each history entry in sessionStorage,
// rather than on history.state itself, because when navigation is driven by
// popstate it's too late to update the scroll position associated with the
Expand Down Expand Up @@ -86,10 +88,12 @@ export function create_client({ target, session, base, trailing_slash }) {

/** @type {import('./types').NavigationState} */
let current = {
// @ts-ignore - we need the initial value to be null
url: null,
branch: [],
error: null,
session_id: 0,
branch: []
stuff: root_stuff,
// @ts-ignore - we need the initial value to be null
url: null
};

let started = false;
Expand Down Expand Up @@ -364,7 +368,7 @@ export function create_client({ target, session, base, trailing_slash }) {
* stuff: Record<string, any>;
* branch: Array<import('./types').BranchNode | undefined>;
* status: number;
* error?: Error;
* error: Error | null;
* routeId: string | null;
* }} opts
*/
Expand All @@ -387,6 +391,8 @@ export function create_client({ target, session, base, trailing_slash }) {
url,
params,
branch,
error,
stuff,
session_id
},
props: {
Expand All @@ -399,7 +405,13 @@ export function create_client({ target, session, base, trailing_slash }) {
result.props[`props_${i}`] = loaded ? await loaded.props : null;
}

if (!current.url || url.href !== current.url.href) {
const page_changed =
!current.url ||
url.href !== current.url.href ||
current.error !== error ||
current.stuff !== stuff;

if (page_changed) {
result.props.page = { error, params, routeId, status, stuff, url };

// TODO remove this for 1.0
Expand Down Expand Up @@ -583,14 +595,14 @@ export function create_client({ target, session, base, trailing_slash }) {
let branch = [];

/** @type {Record<string, any>} */
let stuff = {};
let stuff = root_stuff;
let stuff_changed = false;

/** @type {number | undefined} */
let status = 200;

/** @type {Error | undefined} */
let error;
/** @type {Error | null} */
let error = null;

// preload modules
a.forEach((loader) => loader());
Expand Down
6 changes: 4 additions & 2 deletions packages/kit/src/runtime/client/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,10 @@ export type BranchNode = {
};

export type NavigationState = {
url: URL;
params: Record<string, string>;
branch: Array<BranchNode | undefined>;
error: Error | null;
params: Record<string, string>;
session_id: number;
stuff: Record<string, any>;
url: URL;
};
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,10 @@
};
}
</script>

<script>
import { page } from '$app/stores';
import { goto } from '$app/navigation';
</script>

<button id="reload-button" on:click={() => goto($page.url.toString())}>Reload</button>
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@

<div id="store-stuff">{JSON.stringify($page.stuff)}</div>

<nav><a href="/store/stuff/xxx">xxx</a> <a href="/store/stuff/yyy">yyy</a> <a href="/store/stuff/zzz">zzz</a></nav>
<nav><a href="/store/stuff/xxx">xxx</a> <a href="/store/stuff/yyy">yyy</a> <a href="/store/stuff/zzz">zzz</a> <a href="/store/stuff/foo">foo</a></nav>

<slot />
27 changes: 27 additions & 0 deletions packages/kit/test/apps/basics/src/routes/store/stuff/foo.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<script context="module">
let is_first = true;
export function load() {
if (is_first) {
is_first = false;
throw new Error('uh oh');
}
return {
stuff: {
foo: true
},
props: {
number: 2
}
};
}
</script>

<script>
export let number;
</script>

<h1>stuff - foo</h1>
<p>Number prop: {number}</p>
<a href="/store">store</a>
24 changes: 22 additions & 2 deletions packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1590,11 +1590,11 @@ test.describe.parallel('$app/stores', () => {
await page.goto('/store');

expect(await page.textContent('h1')).toBe('Test');
expect(await page.textContent('h2')).toBe('Calls: 1');
expect(await page.textContent('h2')).toBe(javaScriptEnabled ? 'Calls: 2' : 'Calls: 1');

await clicknav('a[href="/store/result"]');
expect(await page.textContent('h1')).toBe('Result');
expect(await page.textContent('h2')).toBe(javaScriptEnabled ? 'Calls: 1' : 'Calls: 0');
expect(await page.textContent('h2')).toBe(javaScriptEnabled ? 'Calls: 2' : 'Calls: 0');

const oops = await page.evaluate(() => window.oops);
expect(oops).toBeUndefined();
Expand Down Expand Up @@ -1623,6 +1623,26 @@ test.describe.parallel('$app/stores', () => {
);
});

test('should load stuff after reloading by goto', async ({
page,
clicknav,
javaScriptEnabled
}) => {
const stuff1 = JSON.stringify({ name: 'SvelteKit', value: 789, error: 'uh oh' });
const stuff2 = JSON.stringify({ name: 'SvelteKit', value: 123, foo: true });
await page.goto('/store/stuff/www');

await clicknav('a[href="/store/stuff/foo"]');
expect(await page.textContent('#store-stuff')).toBe(stuff1);

await clicknav('#reload-button');
expect(await page.textContent('#store-stuff')).toBe(javaScriptEnabled ? stuff2 : stuff1);

await clicknav('a[href="/store/stuff/zzz"]');
await clicknav('a[href="/store/stuff/foo"]');
expect(await page.textContent('#store-stuff')).toBe(stuff2);
});

test('navigating store contains from and to', async ({ app, page, javaScriptEnabled }) => {
await page.goto('/store/navigating/a');

Expand Down

0 comments on commit e5f8065

Please sign in to comment.