fix(nuxt): sync internal route before calling `page:finish` hook #33707

KazariEX · 2025-11-13T19:01:17Z

🔗 Linked issue

📚 Description

bolt-new-by-stackblitz · 2025-11-13T19:01:21Z

Run & review this pull request in StackBlitz Codeflow.

coderabbitai · 2025-11-13T19:04:30Z

Walkthrough

Route synchronization wiring was changed: the inline pre-hook registration nuxtApp.hook('page:finish', syncCurrentRoute) was removed and synchronization is now exposed via a public-like route object const route = { sync: syncCurrentRoute } as NuxtApp['_route']. A safer last-match check uses to.matched.at(-1). Typing was adjusted: NuxtApp added to imports and _route widened to include an optional sync?: () => void. Page resolve logic was made asynchronous with an await nextTick() and a try/finally around page:finish to guarantee done()/cleanup. Tests were updated to expect fewer route changes and renders.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

packages/nuxt/src/pages/runtime/plugins/router.ts — verify the new route object shape, removal of the pre-hook, correct use of to.matched.at(-1), and that sync is wired and invoked appropriately.
packages/nuxt/src/app/nuxt.ts — confirm _route widening is safe and does not introduce type regressions where stricter RouteLocationNormalizedLoaded was assumed.
packages/nuxt/src/pages/runtime/page.ts — review the new async control flow, await nextTick(), awaited page:finish/page:loading:end hooks, and ensure done()/transition cleanup is always executed.
test/nuxt/nuxt-layout.test.ts — check updated expectations align with runtime behaviour and that reduced re-renders reflect intended semantics.
Search for other _route or _route.sync callsites to avoid race conditions or double-sync scenarios.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: syncing the internal route before calling the page:finish hook, which directly addresses the linked issue.
Description check	✅ Passed	The description references the linked issue #33704, establishing a clear relationship to the changeset and its intent.
Linked Issues check	✅ Passed	The PR implementation directly addresses the issue by syncing the internal route before calling page:finish, ensuring useRoute() reflects the updated route regardless of plugin registration syntax [#33704].
Out of Scope Changes check	✅ Passed	All changes are focused on fixing the route synchronisation timing issue: router.ts updates sync logic, nuxt.ts adds sync method type, page.ts ensures sync before page:finish, and tests reflect expected behaviour adjustments.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

pkg-pr-new · 2025-11-13T19:04:35Z

Open in StackBlitz

@nuxt/kit

npm i https://pkg.pr.new/@nuxt/kit@33707

@nuxt/nitro-server

npm i https://pkg.pr.new/@nuxt/nitro-server@33707

nuxt

npm i https://pkg.pr.new/nuxt@33707

@nuxt/rspack-builder

npm i https://pkg.pr.new/@nuxt/rspack-builder@33707

@nuxt/schema

npm i https://pkg.pr.new/@nuxt/schema@33707

@nuxt/vite-builder

npm i https://pkg.pr.new/@nuxt/vite-builder@33707

@nuxt/webpack-builder

npm i https://pkg.pr.new/@nuxt/webpack-builder@33707

commit: d29438e

codspeed-hq · 2025-11-13T19:21:04Z

CodSpeed Performance Report

Merging #33707 will not alter performance

_{Comparing sinrabansyo:fix/issue-33704 (d29438e) with main (5bf66d4)}

Summary

✅ 10 untouched

packages/nuxt/src/pages/runtime/plugins/router.ts

packages/nuxt/src/pages/runtime/page.ts

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

test/nuxt/nuxt-layout.test.ts (2)

134-142: Consider addressing the TODO and "should be" comments.

Whilst the reduced route changes and renders (from 3→2) reflect an improvement from the route synchronization fix, the inline comments indicate the behaviour is still not optimal. The secondary rerender and extra route change suggest there may be additional opportunities to refine the synchronization timing.

Should this TODO be tracked in a separate issue, or is it within scope for this PR to investigate why routeChanges and renders are 2 instead of the ideal 1?

12-210: Consider adding a test specifically for the page:finish hook behaviour.

Whilst this test verifies that layouts receive correct route values during navigation, it doesn't directly test the bug described in issue #33704: that the page:finish hook was receiving outdated useRoute() values when registered with object syntax.

Consider adding a test case that:

Registers a page:finish hook using both object and function syntax

Navigates between routes

Verifies that both hook registrations receive the updated route path, not the previous one

This would provide explicit regression coverage for the reported issue and make the test suite more comprehensive.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6fe6630 and d29438e.

📒 Files selected for processing (1)

test/nuxt/nuxt-layout.test.ts (3 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: huang-julien
Repo: nuxt/nuxt PR: 29366
File: packages/nuxt/src/app/components/nuxt-root.vue:16-19
Timestamp: 2024-12-12T12:36:34.871Z
Learning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:

```js
const IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null
```

instead of wrapping the import in a computed property or importing the component unconditionally.

Learnt from: GalacticHypernova
Repo: nuxt/nuxt PR: 26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: code

)

fix: remove redundant route sync logic

154e1bc

KazariEX requested a review from danielroe as a code owner November 13, 2025 19:01

github-actions bot added 4.x bug labels Nov 13, 2025

fix: add hook before any async operation to ensure correct order

a2177cd

KazariEX changed the title ~~fix(nuxt): remove redundant route sync logic~~ fix(nuxt): add page:finish hook before any async operation to ensure correct order Nov 13, 2025

KazariEX marked this pull request as draft November 13, 2025 19:13

fix: enforce pre ???

85600a0

KazariEX changed the title ~~fix(nuxt): add page:finish hook before any async operation to ensure correct order~~ fix(nuxt): prepend internal page:finish hook Nov 13, 2025

KazariEX commented Nov 13, 2025

View reviewed changes

packages/nuxt/src/pages/runtime/plugins/router.ts Outdated Show resolved Hide resolved

fix: add _route.sync

fd2d324

KazariEX changed the title ~~fix(nuxt): prepend internal page:finish hook~~ fix(nuxt): sync internal route before calling page:finish hook Nov 15, 2025

refactor: simplify

3eaf2c9

KazariEX marked this pull request as ready for review November 15, 2025 10:02

KazariEX commented Nov 15, 2025

View reviewed changes

packages/nuxt/src/pages/runtime/page.ts Show resolved Hide resolved

OrbisK reviewed Nov 15, 2025

View reviewed changes

packages/nuxt/src/pages/runtime/page.ts Outdated Show resolved Hide resolved

KazariEX added 2 commits November 15, 2025 18:59

fix: await

6fe6630

test: update snapshot

d29438e

coderabbitai bot reviewed Nov 15, 2025

View reviewed changes

danielroe approved these changes Nov 16, 2025

View reviewed changes

danielroe merged commit 6bfef42 into nuxt:main Nov 16, 2025
143 of 145 checks passed

github-actions bot mentioned this pull request Nov 16, 2025

v4.2.2 #33687

Merged

KazariEX deleted the fix/issue-33704 branch November 16, 2025 17:30

danielroe pushed a commit that referenced this pull request Dec 9, 2025

fix(nuxt): sync internal route before calling page:finish hook (#33707

624c226

)

github-actions bot mentioned this pull request Dec 9, 2025

v3.20.2 #33696

Merged

chrisbbreuer mentioned this pull request Dec 9, 2025

chore(deps): update all non-major dependencies stacksjs/ts-maps#479

Closed

coderabbitai bot mentioned this pull request Jan 13, 2026

NuxtPage fails to render route changes when nested inside a NuxtLayout with :name="false" #34074

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(nuxt): sync internal route before calling `page:finish` hook #33707

fix(nuxt): sync internal route before calling `page:finish` hook #33707

KazariEX commented Nov 13, 2025 •

edited

Loading

Uh oh!

bolt-new-by-stackblitz bot commented Nov 13, 2025

Uh oh!

coderabbitai bot commented Nov 13, 2025 •

edited

Loading

Uh oh!

pkg-pr-new bot commented Nov 13, 2025 •

edited

Loading

Uh oh!

codspeed-hq bot commented Nov 13, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

fix(nuxt): sync internal route before calling page:finish hook #33707

fix(nuxt): sync internal route before calling page:finish hook #33707

Conversation

KazariEX commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Linked issue

📚 Description

Uh oh!

bolt-new-by-stackblitz bot commented Nov 13, 2025

Uh oh!

coderabbitai bot commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

pkg-pr-new bot commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codspeed-hq bot commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #33707 will not alter performance

Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix(nuxt): sync internal route before calling `page:finish` hook #33707

fix(nuxt): sync internal route before calling `page:finish` hook #33707

KazariEX commented Nov 13, 2025 •

edited

Loading

coderabbitai bot commented Nov 13, 2025 •

edited

Loading

pkg-pr-new bot commented Nov 13, 2025 •

edited

Loading

codspeed-hq bot commented Nov 13, 2025 •

edited

Loading