[fix] do not call server and update page.data unnecessarily

[dummdidumm]

Fixes #6201
Fixes #6286

Marking this as a draft because the test fails, because the root +layout always reruns on URL changes. Not sure how we should go about that, I'd like to prevent creating a new test app. Should we introduce a top level group?

Also: Not sure if this is the best implementation-wise, or if some things could be made shorter here (saving bytes on the client), or if we can infer this somehow.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

[changeset-bot]

🦋 Changeset detected

Latest commit: a0510ef

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Rich-Harris]

Yeah, I ran into the same problem with #6275 and glossed over it. Wrapping everything in a (group) would certainly make it easier to test anything that relies on bypassing the current route layout.

Implementation-wise, don't we just need to update data if any nodes (server or shared) were invalid, and otherwise leave it be? I'm not sure I understand why we need to treat leaf_uses_server_data differently

[dummdidumm]

If

• the page uses a layout which is already loaded and doesn't need to be reloaded
• the page itself doesn't have a load function

then route.uses_server_data is true and invalid_server_nodes will also have true as the last entry, because the loader is different, because it's a different page. Therefore we also need to separately check if the leaf has a server load function, so that if all nodes leading up to it can be reused, no backend call needs to be made.

[Rich-Harris]

Ah — sounds like the problem is that the invalid_server_nodes reducer is appending true for all new nodes, regardless of whether they have a server-only module or not. In some cases we only need to care about the leaf, but the leaf isn't special — you could equally have a situation where you navigated from /a/b/c to /a/d/e, and both the leaf (c -> e) and the layout (b -> d) changed. The layout changing would cause the same false positive.

So we have two choices: we can either load all the nodes in order to determine whether they have a server-only module (which causes a delay), or we can encode uses_server_data in the manifest for each node individually (which increases the manifest size). If we wanted to minimise the effect on the manifest size we could encode it in a bitmask, perhaps, though that would limit route depth (including groups) to 31.

[dummdidumm]

Oh right, any layout along the way could also not have a load function. If we use the ~ trick for all layouts and not only the leaf, each layout encodes "I have a server load function" and the leaf no longer needs to encode that for itself and all parent, rather only itself. I feel this is better than a perf hit on loading. Let me try that ..

[Rich-Harris]

ah of course! ~ on layouts makes a ton of sense

[dummdidumm]

I adjusted the root layout so it no longer reruns, which made the tests I've written possible/correct. Wondering why the CI is so flaky recently though ... locally it ran fine.

[Rich-Harris]

The adapter-static tests are failing for me locally — I would guess there's an infinite reload loop involved, though I haven't investigated yet