feat: prevent race condition dev-only error - #2789
Conversation
✅ Deploy Preview for vue-router canceled.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughRouterView now guards the ChangesRouterView devtools metadata
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to The PR adds a localized null guard in a dev-only RouterView path, preventing a first-render crash when an instance is absent while preserving existing behavior for valid instances; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
The fix is fine indeed but sounds like another bug worth digging. Do you have a repro without nuxt? |
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2789 +/- ##
=======================================
Coverage 85.95% 85.95%
=======================================
Files 74 74
Lines 5895 5895
Branches 1893 1894 +1
=======================================
Hits 5067 5067
Misses 732 732
Partials 96 96 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Thanks for your reply At the moment, I have only been able to reproduce it in a Nuxt 4 + @nuxt/content setup. I haven't yet found a reliable reproduction using vue-router alone. |
vue-router 5.2.0 has a dev-only race in RouterView.ts: it assigns instance.__vrv_devtools = info without checking instance is non-null, crashing elevator-ui with "null is not an object (evaluating 'instance.__vrv_devtools = info')" on first render. Fixed upstream in vuejs/router#2789, released in vue-router 5.3.0. Bump elevator-ui's declared range from ^5.2.0 to ^5.3.1 and refresh the lockfile. Not a code smell in this codebase -- an upstream dependency bug.

What kind of change does this PR introduce?
Bug fix.
Did you add tests for your changes?
No. Reliably reproducing the null state inside
@vue/test-utils+happy-domrequires an SSR/hydration timing race that does not occur in the current test harness. The fix is a one-line defensivenullcheck that has no impact wheninstanceis already defined, so a regression test would only exercise the untouched code path. Happy to add one if a reviewer can suggest a stable reproduction pattern.If relevant, did you update the documentation?
No documentation change needed.
Summary
Under certain rendering conditions — observed in a Nuxt 4 SSR app with
@nuxt/content—component.ref.iinRouterView.tsisnullwhen the dev-only devtools branch runs. The unguardedinstance.__vrv_devtools = infoassignment then throws:crashing the app on first render.
Change
internalInstances.forEach(instance => { // @ts-expect-error - instance.__vrv_devtools = info + if (instance) instance.__vrv_devtools = info })Why this is safe
__DEV__ || __FEATURE_PROD_DEVTOOLS__andisBrowser, so production builds are unaffected.instanceis truthy the behavior is unchanged.instanceisnull, the devtools info is simply not attached to that entry.RouterViewre-runs this code on subsequent renders / re-mounts, so no persistent state is lost.Reproduction steps
@nuxt/content(v3.15.2) and register it innuxt.config.ts:app/pages/index.vue:npm run devand openhttp://localhost:3000.TypeErrorshown above.Environment: Nuxt 4.5.2,
@nuxt/content3.15.2, vue-router 5.2.0, Vue 3.5.41.Non-native English speaker; apologies for any awkward phrasing.
Summary by CodeRabbit