-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
fix(watch): use maximum depth for duplicates #13434
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Watcher
participant ReactiveArray
User->>Watcher: Set up watchers with deep: 1, 2, 3, 4
User->>ReactiveArray: Mutate nested properties at various depths
ReactiveArray-->>Watcher: Notify relevant watchers based on deep level
Watcher-->>User: Trigger callbacks for matching deep levels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code Graph Analysis (1)packages/reactivity/src/watch.ts (3)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
While recursing for deep watchers, we keep track of which objects have already been 'seen', to avoid duplicated effort and prevent cycles. But currently we don't take account of the depth.
If we encounter an object twice during the traversal, with different values for
depth
, we need to traverse it at the greaterdepth
.I think the fix for this is relatively simple, using a
Map
instead of aSet
to store the seen objects. If we encounter an object again then we may need to traverse it again if the previous traversal wasn't deep enough.While this may lead to wasted effort, traversing the same object multiple times, in practice it happens very rarely. It needs a pretty specific data structure and the value passed for
deep
needs to be an unusually large but finite number. The simplest broken example I could find usesdeep: 3
:I did briefly test the performance impact, but I didn't see a noticeable difference.
#13433 would also fix this problem, because it processes the items in depth order. But I think that PR may prove difficult to merge.
Summary by CodeRabbit
Tests
Refactor