Skip to content

Commit

Permalink
update page store more conservatively
Browse files Browse the repository at this point in the history
  • Loading branch information
Rich-Harris committed Apr 15, 2022
1 parent 48bd321 commit 49f37b8
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 21 deletions.
31 changes: 21 additions & 10 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,11 +391,12 @@ export function create_client({ target, session, base, trailing_slash }) {
url,
params,
branch,
error,
stuff,
session_id
},
props: {
components: filtered.map((node) => node.module.default),
page: { error, params, routeId, status, stuff, url }
components: filtered.map((node) => node.module.default)
}
};

Expand All @@ -400,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 @@ -584,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;
};
12 changes: 5 additions & 7 deletions packages/kit/test/apps/basics/src/routes/store/index.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,11 @@
});
const unsubscribe = page.subscribe(() => {
if (calls === 0) {
calls++;
session.update(($session) => ({
...$session,
calls
}));
}
calls++;
session.update(($session) => ({
...$session,
calls
}));
});
onDestroy(unsubscribe);
Expand Down
4 changes: 2 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

0 comments on commit 49f37b8

Please sign in to comment.