LayoutData is reset even when unchanged

[CaptainCodeman]

Describe the bug

The /__data.json file is re-requested on every route change, even if the data is unchanged and not invalidate()'d. The data prop is then re-assigned which triggers updates within the app.

Response:

{"type":"data","nodes":[{"type":"skip"},null]}

It's less noticeable during dev, when things are local, but with a proper network request the latency can be noticeable - simulating a longer network request makes it apparent and it is also triggering a change on the Layout / Page data which is triggering store re-calcs and (I presume) potential re-renders.

Reproduction

Attach an extra hook to highlight the latency issue:

import type { Handle } from '@sveltejs/kit'

export const handle: Handle = async ({ event, resolve }) => {
  await delay(500)

  return resolve(event)
}

async function delay(ms: number) {
  await new Promise(resolve => setTimeout(resolve, ms))
}

Or, use that dev-tool network throttling option that I just remembered about while typing this ... 😊

Logs

No response

System Info

System:
    OS: macOS 12.5.1
    CPU: (6) x64 Intel(R) Core(TM) i5-8500B CPU @ 3.00GHz
    Memory: 1.89 GB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.14.0 - /usr/local/bin/node
    npm: 8.3.1 - /usr/local/bin/npm
  Browsers:
    Brave Browser: 102.1.39.111
    Chrome: 104.0.5112.101
    Chrome Canary: 107.0.5260.0
    Firefox: 103.0.2
    Safari: 15.6.1
    Safari Technology Preview: 16.0
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.66 
    @sveltejs/adapter-node: next => 1.0.0-next.87 
    @sveltejs/kit: next => 1.0.0-next.440 
    svelte: ^3.49.0 => 3.49.0 
    vite: ^3.0.9 => 3.0.9

Severity

serious, but I can work around it

Additional Information

Related to #6201 (couldn't add a comment to it)

[CaptainCodeman]

Probably unrelated at this point after so many changes, but its this issue again: #5247

[Rich-Harris]

Is this a dupe of #6201 or a separate issue? If separate, it needs a repro

[CaptainCodeman]

I'm not sure, that seemed more about the "making the network request part" which is obviously related, but even if a request is made if it says "no update" I'd expect it to avoid setting the data, so maybe they are two separate issues (just in case a fix for that only addresses the network request part).

I'll work on a repro.

[Rich-Harris]

Ah, gotcha — didn't fully grok it first time. Yeah, it does seem like valid data should be left alone even if SvelteKit hit the server and the load function was run (which in this case I suspect it shouldn't have been)

[CaptainCodeman]

Ah, I think the problem may just be between the chair and the keyboard ...

It's actually the $page store that was updating, which is of course expected as the route changes - I was using a derived store that plucked an object off $page.data for use by other stores in other parts of the app as a slightly easier way to code it than destructuring and then setting it in the /+layout.svelte file (which also needed the store to be writable). I was expecting that derived store to not update if things hadn't changed which I could have sworn was an approach I've used before to kind of "debounce" changes to only trigger for a subset of the data.

So, maybe that updating is expected, or should $page.data keep the same data ref unchanged if the /__data.json doesn't change anything?

[Rich-Harris]

Keeping $page.data the same if no load functions were invalidated seems like reasonable behaviour, though that really needs to apply to the whole $page object. In theory that should already happen, but likely isn't in this case because of the false positive around __data.json:

kit/packages/kit/src/runtime/client/client.js

Lines 437 to 441 in 163efb3

	const page_changed =
	!current.url || url.href !== current.url.href || current.error !== error || data_changed;
	if (page_changed) {
	result.props.page = { error, params, routeId, status, url, data };

[CaptainCodeman]

Yes, it feels a little strange to have the same data in two places esp if they are then updating at different times, but ideally it wouldn't be that way for all the same reasons as before (unwanted store calculations / re-renders).

[dummdidumm]

The problem with calling the backend is that we don't know that the new page doesn't have a load function. We only know "this route uses server data somewhere along the way", but we don't know "this leaf doesn't use a load function and everything leading up to it is unchanged so you can skip requesting the data". This feels more related to #6201.

The problem with $page.data is that we need to construct a new object for any changes to be present. What we could add is some logic which goes like "nothing was added or removed from page.data, so we can keep it".