fix: prevent circular reference stack overflow in payload.__hints#289
Conversation
commit: |
📝 WalkthroughWalkthroughThe PR reduces the depth of reactivity tracking for data containing DOM element references stored in Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/third-party-scripts/plugin.client.ts (1)
62-76:⚠️ Potential issue | 🟠 Major
shallowRefmutations won't trigger reactivity in downstream computed properties.The
scriptsref is exposed directly to consumers viauseHostThirdPartyScripts(), which use it in computed properties that depend onscripts.value. At lines 69 and 75 in the plugin, in-place mutations (array.push()andobject.loaded = true) don't notify subscribers when usingshallowRef, preventing the UI from updating when scripts are added or theirloadedstate changes.Call
triggerRef(scripts)after each mutation to force reactivity:Proposed fix
-import { defineNuxtPlugin, shallowRef, useNuxtApp } from '#imports' +import { defineNuxtPlugin, shallowRef, triggerRef, useNuxtApp } from '#imports' nuxtApp.hook('hints:scripts:added', (script: HTMLScriptElement) => { scripts.value.push({ element: script, loaded: false }) + triggerRef(scripts) }) nuxtApp.hook('hints:scripts:loaded', (script: HTMLScriptElement) => { const existingScript = scripts.value.find(s => s.element === script) if (existingScript) { existingScript.loaded = true + triggerRef(scripts) } else { logger.warn(...) scripts.value.push({ element: script, loaded: true }) + triggerRef(scripts) } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/third-party-scripts/plugin.client.ts` around lines 62 - 76, The scripts shallowRef is mutated in-place (via scripts.value.push(...) in the nuxtApp.hook('hints:scripts:added') handler and existingScript.loaded = true in the nuxtApp.hook('hints:scripts:loaded') handler) which doesn't notify consumers; after each mutation call triggerRef(scripts) to force reactivity so computed consumers (e.g. useHostThirdPartyScripts() users) update — add triggerRef(scripts) immediately after the push and immediately after setting existingScript.loaded = true.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/runtime/third-party-scripts/plugin.client.ts`:
- Around line 62-76: The scripts shallowRef is mutated in-place (via
scripts.value.push(...) in the nuxtApp.hook('hints:scripts:added') handler and
existingScript.loaded = true in the nuxtApp.hook('hints:scripts:loaded')
handler) which doesn't notify consumers; after each mutation call
triggerRef(scripts) to force reactivity so computed consumers (e.g.
useHostThirdPartyScripts() users) update — add triggerRef(scripts) immediately
after the push and immediately after setting existingScript.loaded = true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 653eaeea-3d5c-4773-bf8e-f3ad15875899
📒 Files selected for processing (3)
src/runtime/hydration/composables.tssrc/runtime/third-party-scripts/plugin.client.tssrc/runtime/web-vitals/plugin.client.ts
|
Glad to see this issue, I just got the same issue today and I didn't understand what going wrong. |
|
I'll make a patch release tonight, in the meantime, you can use the nightly release |
🔗 Linked issue
resolves #284 caused by #240
📚 Description
Problem
PR #240 moved hints data from
nuxtApp.__hintstonuxtApp.payload.__hints.Since
payloadis reactive, Vue'straverse()recurses infinitely on circular references (component instances, vnodes, DOM elements), causingRangeError: Maximum call stack size exceeded.This only happens when the application has for example hydration error's and the hints module is displaying them.
Fix
With this fix all deep-traverses should be fixed. The plain [] inside the reactive payload and the ref() to shallowRef()
instanceandvnodewithmarkRaw()before storing in payloadshallowRef()instead ofref()(metrics contain DOM element references)shallowRef()instead ofref()(storesHTMLScriptElementreferences)