-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(nuxt): page hydration and double load #7940
fix(nuxt): page hydration and double load #7940
Conversation
Use ref count to wait for all existing suspense instance to finish before the `app:suspense:resolve` is called
|
β Deploy Preview for nuxt3-docs canceled.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great to me - thank you so much π
I've made a couple of minor changes and one fix (to ensure this works for layouts that are children of <NuxtPage>
); let me know if you spot any issues.
P.S. really appreciate the work on the tests...
Looks fine to me except I am wondering what is this for 21d872c#diff-ef9ac7f796e2b5d9a4a232b098d7ce5dd92610191d35dfaf10612a76bfe532ddR50 Is this exist to ensure that layout used inside page changes when writing things like Also, this didn't seem to change any test results, is there a test for it? |
Yes, so, for example if you are changing page which has a layout within it, it doesn't force rerender the existing page with the new layout before suspense resolves. And yes, we should add a test for that case. I'll do that later unless you get to it first. |
Yea, I think it would makes more sense in anthoer PR instead of this due to following reason.
|
I have great sympathy for your desire to push it to another PR, but I think it's not as complex as you are worried it will be. It's just a matter of creating two files:
I do think it would be good to add tests for this in this PR as it would be good to check that 21d872c is doing what it's meant to do, and that we don't accidentally regress the fix. |
https://stackblitz.com/edit/github-42bhq4?file=layouts%2Ftest3.vue,pages%2Findex.vue I think it will still also change the outer layout even you already set |
would love to see this pr merged and released π |
The only thing it needs is the test I mentioned previously. I'm on it. |
any news on this ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
Thank you very much! It would be nice to have a way to detect how much kb add pull requests to the Nuxt default bundle size. |
So, just out of curiosity (because I ran into nuxt/nuxt#14573 as well) β when is the next RC scheduled to be available? π₯Ή |
Until then, you can try this out in the edge channel. |
Thank you, will take a look. π₯³ |
π Linked issue
Fix nuxt/nuxt#14839, fixes nuxt/nuxt#14573, closes #7400
β Type of change
π Description
This PR is a remake of #7400
Besides of fix and workaround in #7400.
A remaining issue is temporary blank page when switching between layout and navigate at the same time.
But that isn't fixable without vuejs/core#6736.
So it isn't handled here.
π Checklist