Skip to content
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

__layout props update on every route change #5247

Closed
CaptainCodeman opened this issue Jun 22, 2022 · 4 comments · Fixed by #5671
Closed

__layout props update on every route change #5247

CaptainCodeman opened this issue Jun 22, 2022 · 4 comments · Fixed by #5671

Comments

@CaptainCodeman
Copy link
Contributor

CaptainCodeman commented Jun 22, 2022

Describe the bug

Not sure if this is a bug or should just be a discussion, but it was a surprise. When loading some data once in a parent __layout I expected that data to then remain static. But the props appear to be re-assigned on every navigation, even for querystring parameter changes. This plays havoc with any use:actions which think things have been updated when they haven't (which may contain image gallery layout code, canvas setup etc... that I only want to re-run when the data actually changes)

Reproduction

Simplified example to show the behavior:
https://github.com/CaptainCodeman/sveltekit-layout-props-example

Run the project and navigate around. Note the prop changes and action update console logs, even though the load function has only executed once:

Screen Shot 2022-06-22 at 12 38 08 PM

I wouldn't expect the data prop to keep changing or for the use:action to keep being updated, but they are in both the __layout and an imported component.

Logs

No response

System Info

System:
    OS: macOS 12.4
    CPU: (6) x64 Intel(R) Core(TM) i5-8500B CPU @ 3.00GHz
    Memory: 335.20 MB / 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: 102.0.5005.115
    Chrome Canary: 105.0.5134.0
    Edge: 102.0.1245.41
    Firefox: 101.0.1
    Safari: 15.5
    Safari Technology Preview: 16.0
  npmPackages:
    @sveltejs/adapter-static: next => 1.0.0-next.34 
    @sveltejs/kit: next => 1.0.0-next.352 
    svelte: ^3.44.0 => 3.48.0

Severity

annoyance

Additional Information

No response

@CaptainCodeman CaptainCodeman changed the title __layout props update on every URL change __layout props update on every route change Jun 22, 2022
@CaptainCodeman
Copy link
Contributor Author

NOTE: the data is the same object reference each time, so I'm actually kind of surprised that it updates as it does but I guess it's doing it on the assignment operation and ignoring any reference check (which I thought Svelte did to avoid unnecessary updates).

I can get it to give the expected behavior by checking for the reference change myself. i.e. return dataraw from the load function and check for it differing from a local variable:

export let dataraw

let data

$: if (data !== dataraw) data = dataraw

@jacob-8
Copy link
Contributor

jacob-8 commented Jul 14, 2022

should just be a discussion

I appreciate that the load function runs every time queryparams change. This is important as it gives a lot of flexibility for pages where one might want to show multiple panels side by side based on queryparams. For example:
.../read?pane1=doc1234&pane2=doc34&pane3=doc55

@CaptainCodeman
Copy link
Contributor Author

I appreciate that the load function runs every time queryparams change. This is important as it gives a lot of flexibility for pages where one might want to show multiple panels side by side based on queryparams

I agree to a point - if the __layout was making use of queryparams (ideally the specific ones that had changed) then I'd expect it to re-run, but when the load function is purely dependent on a route param that hasn't changed it appears to behave differently to the load function in a page.

@Co1eCas3
Copy link

Co1eCas3 commented Jul 16, 2022

I believe this issue I've run into is related to this discussion, and I do believe it to be an issue.

I've discovered that when altering the state of props that are import from a load function in a layout, those state changes are not retained when navigating. The load function does not re-run at all, nor do any function that may be responsible for altering said state. The props simply are reverted back to their initial state on first load for no apparent reason. This is irregardless of any load input that may or may not be used.

Please refer to this example site I've created for a clearer picture. Here's the repo.

While @CaptainCodeman calls it a mere 'annoyance', I feel this could be particularly detrimental in some scenarios, especially if/when layout endpoints are finally introduced.

dummdidumm added a commit that referenced this issue Jul 22, 2022
Fixes #5247 by not adding unchanged layout or page props to the root props
Rich-Harris added a commit that referenced this issue Jul 25, 2022
* [fix] Prevent needless prop updates causing rerenders

Fixes #5247 by not adding unchanged layout or page props to the root props

* enhance test

* spaces -> tabs, and add explanatory comment so we dont later delete it

Co-authored-by: Simon Holthausen <[email protected]>
Co-authored-by: Rich Harris <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants