fix(nuxt): render component-less parent routes during client-side nav#35036
Conversation
|
|
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/nuxt/nuxt-page.test.ts`:
- Around line 738-757: The parent route added with router.addRoute({ path:
'/parent-34967', ... }) has no name so it never gets removed; capture the
teardown function returned by router.addRoute (e.g., const removeParent =
router.addRoute(...)) when adding the parent route and call that function in
afterEach (alongside the existing router.removeRoute('index-34967') and
router.removeRoute('child-34967')) to fully unregister the unnamed parent route;
ensure removeParent is declared in the test scope so afterEach can call it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e4f4ad01-6397-4cd3-bf7c-89154efea210
📒 Files selected for processing (2)
packages/nuxt/src/pages/runtime/page.tstest/nuxt/nuxt-page.test.ts
@nuxt/kit
@nuxt/nitro-server
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
Merging this PR will not alter performance
Comparing Footnotes
|
posva
left a comment
There was a problem hiding this comment.
Seems good. I tried to simplify the test a bit, I'm aware the same changes can be applied in the file but I had trouble running the unit tests, they were running in a loop and the lsp server was crashing all the time too.
I feel like the page component is too complicated. I know a lot of the complexity comes from supporting Suspense but it feels odd we need all these extra things to handle it.
|
weird! i would love to move some of the complexity out (into vue-router if you think worth it? specifically the bit that ensures each suspense fork has access to the route that it is rendering) |
|
I think some of the complexity is worth moving to vue-router but it shouldn't impact users that do not rely on Suspense. I also feel like Suspense has never reached maturity. In the upcoming version, RouterLink and RouterView are not installed by the router plugin, so it also opens more possibilities to have different versions, some lighter than others |
…#35036) Co-authored-by: Eduardo San Martin Morote <posva13@gmail.com>
…#35036) Co-authored-by: Eduardo San Martin Morote <posva13@gmail.com>
🔗 Linked issue
resolves #34967
📚 Description
this bug was introduced in #21823 - some time ago! - in Nuxt v3.6.2...