framework
framework copied to clipboard
fix(nuxt): track suspense status so we can share single state
🔗 Linked issue
resolves #7337
❓ Type of change
- [ ] 📖 Documentation (updates to the documentation or readme)
- [x] 🐞 Bug fix (a non-breaking change that fixes an issue)
- [ ] 👌 Enhancement (improving an existing functionality like performance)
- [ ] ✨ New feature (a non-breaking change that adds functionality)
- [ ] ⚠️ Breaking change (fix or feature that would cause existing functionality to change)
📚 Description
This PR:
- resolves a bug where we generated 'too many' Suspense components, meaning that
isHydratingwasfalsein nested child pages on initial load (which has bad consequences for data fetching, as we make decisions based on whether it's the first load or not). - works around a vue core issue where nothing under an async component is correctly wrapped into a parent suspense (so this would include all pages with a
<NuxtLayout>) meaning thatisHydratingis incorrectly false for these entire trees.
📝 Checklist
- [x] I have linked an issue or discussion.
- [ ] I have updated the documentation accordingly.
Deploy Preview for nuxt3-docs canceled.
| Name | Link |
|---|---|
| Latest commit | f6140997a610ee929faf6c724e1bcb77531e4115 |
| Latest deploy log | https://app.netlify.com/sites/nuxt3-docs/deploys/632b138d86dfbe0008d7c37d |
(added pending to land after RC.10)
@danielroe Would you please help to resolve conflicts? Will do a quick test afterwards to merge 🚀
Sure 👍 I am also working on resolving https://github.com/nuxt/nuxt.js/issues/14573, which may touch some of these files too, but I think this is safe to merge and we can iterate.
https://github.com/nuxt/framework/pull/7400/files#diff-38b334f71e61b2fb35956472399fb0c87ca4a9d84ef6f99c570c973115f20f0fR49
Isn't change of suspense wrapper one of the reason that page loaded twice?
https://stackblitz.com/edit/github-fktkzl-zxyvww?file=src%2FApp.vue
I don't think vue can keep the page instance if the wrapper changed.
@mmis1000 Yes, it is, and https://github.com/nuxt/nuxt.js/issues/14573 exists to track it. I don't believe it's affected by this PR; please let me know if you think otherwise.
@mmis1000 Yes, it is, and nuxt/nuxt.js#14573 exists to track it. I don't believe it's affected by this PR; please let me know if you think otherwise.
I think use of any additional reactive property that does not change at the same time of route in the render function of NuxtPage component is a entirely bad idea. (Just like in this PR, which introduced more reactive state.). Because any structure change of parent of page component WILL RELOAD the whole page.
This PR deliberately doesn't make it reactive. So it should only change the suspense structure if there is already a rerender happening.
This PR deliberately doesn't make it reactive. So it should only change the suspense structure if there is already a rerender happening.
https://stackblitz.com/edit/github-gagmal-vjqqo3?file=pages%2Fb%2F[id].vue
That line alone is already enough to cause bug.
- Navigate to /b/b1
- reload page to get a SSR's initial page
- click
b2to go to /b/b2 - page is reloaded even it shouldn't (The page key is already specified as
bviadefinePageMeta) a. Notice: There is a new 'Suspense' mark on the nested page component after this navigation. - navigate to b1 again
- it works correctly this time, page is not reloaded
You're right, it does cause an unnecessary rerender. We may pend this PR as I have local work-in-progress that doesn't cause it, and it makes sense to resolve nuxt/nuxt.js#14573 at the same time.
https://github.com/mmis1000/nuxt-framework/commit/6bb08590805a16d514dff5f63262af9e1acae796
I think one of potential fix would be using a counter to count how many <Suspense> we created, and delay the resolve until every one resolved?
By the way, is the isNested check there to workaround some issue? It seems that stop all redirection between sibling of nested routes (/a/a1 -> /a/a2) to emit page:start and page:finish even there is a page load.