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

client side navigation incorrectly re-uses data from previous request #8272

Closed
ivanhofer opened this issue Dec 28, 2022 · 5 comments · Fixed by #8893
Closed

client side navigation incorrectly re-uses data from previous request #8272

ivanhofer opened this issue Dec 28, 2022 · 5 comments · Fixed by #8893
Labels
bug Something isn't working

Comments

@ivanhofer
Copy link
Contributor

Describe the bug

The load_route function re-uses data from a previous result if loaders array does not have any data to load at a specific index. In my case I first load data where 5 different loaders are involved. On the next page (which includes a layout reset) only 3 loaders are involved. The last one does not provide any data and SvelteKit uses the data from the previous request, which is wrong since that data should not be involved in the rendering process becasue of the layout reset.

image

those logs were created by adding console.log(1, url.pathname, branch.map(({ data }) => data)); here.

Marked in red is the data at the specific index that causes the issue. The first log comes from the initial page load. The second log is a client-side navigation and the third on is a page reload. Marked in orange is the data that get's set incorrectly from the previous response. It should actully be null like in log line nr 3.

My specific use case works if I delete this line. But I don't fully understand everything that happens here. Something in this invalidation logic is wrong. Or maybe the code where branch_promises gets set.

The whole data logic seems really buggy (see #8157).

Reproduction

I'm sorry, I was not able to reproduce this in a fresh repo in the limited time I had.

Let me know if the description isn't enough and I will try to find some time to create a stackblitz example.

Logs

No response

System Info

System:
    OS: Linux 5.15 Ubuntu 20.04.5 LTS (Focal Fossa)
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i7-11370H @ 3.30GHz
    Memory: 1.48 GB / 9.72 GB
    Container: Yes
    Shell: 5.8 - /usr/bin/zsh
  Binaries:
    Node: 16.18.1 - ~/.volta/tools/image/node/16.18.1/bin/node
    Yarn: 1.22.17 - ~/.volta/tools/image/yarn/1.22.17/bin/yarn
    npm: 8.19.2 - ~/.volta/tools/image/node/16.18.1/bin/npm
  Browsers:
    Chrome: 108.0.5359.98
  npmPackages:
    @sveltejs/adapter-node: 1.0.0 => 1.0.0 
    @sveltejs/kit: 1.0.0 => 1.0.0 
    svelte: 3.55.0 => 3.55.0 
    vite: ^4.0.1 => 4.0.1

Severity

serious, but I can work around it

Additional Information

My current workaround is it to create an empty +page.server.ts. The file does not need to export a load function. The simple presence of that file is enough (see #7967).

@dummdidumm dummdidumm added the bug Something isn't working label Jan 12, 2023
@bbozzay
Copy link

bbozzay commented Jan 18, 2023

I'm having the same issue where client-side navigations re-use data from the previous route, but the workaround (creating an empty +page.server.ts) doesn't work for me.

update: I finally found my issue and I'm not sure it's related:

The content of my page uses an {#each} loop to map blocks in PageData to components. Client-side navigation would result in some page data changing, but not all of the content would change, so the page would have some content from the previous route. Adding a key to the loop fixed this.

The other issue was with the sticky anchor navigation component not updating with client-side navigation. I fixed this with a reactive variable. I didn't expect to have to use reactive variables with pre-rendered content. Maybe this is unintended and related to #8302

@ivanhofer
Copy link
Contributor Author

@bbozzay for me your issues don't look like they are related to this one.

The content of my page uses an {#each} loop to map blocks in PageData to components. Client-side navigation would result in some page data changing, but not all of the content would change, so the page would have some content from the previous route. Adding a key to the loop fixed this.

Sounds like intended. This is how Sveltes #each work. If you don't provide a key, Svelte dosn't really know if and what has changed.

The other issue was with the sticky anchor navigation component not updating with client-side navigation. I fixed this with a reactive variable. I didn't expect to have to use reactive variables with pre-rendered content. Maybe this is unintended and related to #8302

Without knowing your code, I can only assume that this is also something that works like intended. If you don't use reactive statements, the DOM will not update if your data changes.

@ivanhofer
Copy link
Contributor Author

Note: #7967 got fixed, so now you have to add an empty load function for the workaround

@coyotte508
Copy link
Contributor

coyotte508 commented Jan 29, 2023

https://github.com/coyotte508/sveltekit-load-bug - I think I encountered the same issue, here's a minimal repo

Here's a simpler description:

When +layout.server.ts and +page.server.ts return a property with the same name (eg title), $page.data.title is the value from +page.server.ts. So far so good.

But if I navigate to another page without +page.server.ts, then $page.data.title keeps being the old page's title, instead of the title from +layout.server.ts.

@coyotte508
Copy link
Contributor

coyotte508 commented Feb 5, 2023

After digging into it, the backend will return {type: "skip"} on the __data.json for the page's data when there is no +page.server.ts. The frontend will then keep the old page's data in cache, instead of erasing it.

coyotte508 added a commit to coyotte508/kit that referenced this issue Feb 5, 2023
fixes sveltejs#8272

When going from a page with server load to a page without server load, the cache wasn't purged properly
coyotte508 added a commit to coyotte508/kit that referenced this issue Feb 5, 2023
fixes sveltejs#8272

When going from a page with server load to a page without server load, the cache wasn't purged properly
dummdidumm added a commit that referenced this issue Feb 7, 2023
fixes #8272

When going from a page with server load to a page without server load, the cache wasn't purged properly

---------

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants