-
-
Notifications
You must be signed in to change notification settings - Fork 9k
feat(reactivity): add 'equals' option to watch for custom equality checks #14256
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
📝 WalkthroughWalkthroughAdded an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/runtime-core/__tests__/apiWatch.spec.ts (1)
2064-2091: Add assertions after the second value assignment.The test demonstrates the workaround for the issue, but it's missing final assertions after line 2091 to verify that
cleanupCountandeffectCountremain unchanged when the semantically identical value is assigned again.🔎 Suggested assertions
state.value = { id: 2 } await nextTick() + // Should not trigger because values are semantically equal + expect(effectCount).toBe(1) + expect(cleanupCount).toBe(0) })
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/reactivity/src/watch.tspackages/runtime-core/__tests__/apiWatch.spec.tspackages/runtime-core/src/apiWatch.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/runtime-core/__tests__/apiWatch.spec.ts (4)
packages/reactivity/src/ref.ts (1)
ref(61-63)packages/reactivity/src/watch.ts (1)
watch(121-337)packages/runtime-core/src/apiWatch.ts (1)
watch(132-145)packages/runtime-core/src/scheduler.ts (1)
nextTick(61-67)
🔇 Additional comments (3)
packages/runtime-core/src/apiWatch.ts (1)
53-53: LGTM! Type definition is clear and consistent.The
equalsoption is properly typed as optional and follows Vue's pattern for watch options. Theanytypes are pragmatic for a general-purpose equality comparator, though they sacrifice type safety.packages/runtime-core/__tests__/apiWatch.spec.ts (1)
2092-2121: LGTM! Test properly validates the equals option functionality.The test correctly verifies that when a custom
equalsfunction returnstrue, both the callback and cleanup are suppressed. The test cases cover the essential scenarios: initial change triggering the callback, and subsequent semantically-equal assignments being skipped.packages/reactivity/src/watch.ts (1)
53-53: LGTM! Consistent with the runtime-core API definition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/reactivity/src/watch.ts (1)
244-257: Consider type-safe generics for theequalsfunction signature.The current signature
equals?: (a: any, b: any) => booleanloses type information about the watched values. While consistent with existing patterns in the codebase, consider enhancing type safety in a future iteration by makingWatchOptionsgeneric over the source type(s) soequalsreceives properly typed parameters.This would provide better IntelliSense and catch type mismatches at compile time.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/reactivity/src/watch.tspackages/runtime-core/__tests__/apiWatch.spec.tspackages/runtime-core/src/apiWatch.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/runtime-core/tests/apiWatch.spec.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/reactivity/src/watch.ts (1)
packages/shared/src/general.ts (1)
hasChanged(143-144)
🔇 Additional comments (3)
packages/runtime-core/src/apiWatch.ts (1)
53-58: LGTM! Clear documentation of the new equals option.The JSDoc accurately describes the behavior and precedence of the
equalsoption. The documentation correctly states thatequalstakes precedence over the default equality check,deep, andforceTrigger, which aligns with the implementation inwatch.ts.packages/reactivity/src/watch.ts (2)
53-53: LGTM! Type definition matches the public API.The
equalsoption is correctly added to theWatchOptionsinterface, maintaining consistency with the definition inruntime-core/src/apiWatch.ts.
253-254: Good: Inline comment clarifiesequalsprecedence.The inline comment addresses the previous review feedback by clearly documenting that
equalsbypassesdeepandforceTriggerlogic when provided. This helps developers understand the precedence behavior.
| const isChanged = isMultiSource | ||
| ? (newValue as any[]).some((v, i) => | ||
| options.equals | ||
| ? !options.equals(v, oldValue[i]) | ||
| : hasChanged(v, oldValue[i]), | ||
| ) | ||
| : options.equals | ||
| ? !options.equals(newValue, oldValue) | ||
| : hasChanged(newValue, oldValue) | ||
| // If equals is provided, it fully controls the trigger decision, | ||
| // bypassing deep and forceTrigger logic. | ||
| const shouldTrigger = options.equals | ||
| ? isChanged | ||
| : deep || forceTrigger || isChanged |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling around user-provided equals function calls.
If the user-provided equals function throws an exception (e.g., due to accessing properties on null/undefined, or internal logic errors), the entire watcher job will fail, potentially breaking reactive updates and leaving the component in an inconsistent state.
Wrap the equals invocations in try-catch blocks and handle errors gracefully, similar to how other user callbacks are handled via the call helper throughout this codebase.
🔎 Example approach for error handling
Consider wrapping equals calls:
const isChanged = isMultiSource
? (newValue as any[]).some((v, i) =>
options.equals
- ? !options.equals(v, oldValue[i])
+ ? !safeEquals(options.equals, v, oldValue[i])
: hasChanged(v, oldValue[i]),
)
: options.equals
- ? !options.equals(newValue, oldValue)
+ ? !safeEquals(options.equals, newValue, oldValue)
: hasChanged(newValue, oldValue)Where safeEquals is a helper that wraps the call in try-catch and uses the call helper for error handling, or falls back to hasChanged on error.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/reactivity/src/watch.ts around lines 244 to 257, user-supplied
equals calls are invoked directly and can throw, which would crash the watcher;
wrap every equals invocation (both in the multi-source some(...) comparator and
the single-source branch) in a small try-catch wrapper (or create a safeEquals
helper) that calls the equals using the existing call helper for consistent
error handling, and on exception fall back to using hasChanged (or treat as
changed) so the watcher continues to function; ensure the computed isChanged and
shouldTrigger use the safe result and preserve existing deep/forceTrigger
behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/reactivity/src/watch.ts
🔇 Additional comments (3)
packages/reactivity/src/watch.ts (3)
53-53: LGTM!Clean API addition that follows the existing optional property pattern in
WatchOptions.
259-267: LGTM!The
isChangedcalculation correctly handles both single and multi-source watchers. TheshouldTriggerlogic appropriately lets customequalstake full control of the trigger decision when provided, which aligns with user expectations when they explicitly opt into custom equality semantics. The inline comment clearly documents this design choice.
268-268: LGTM!The conditional correctly uses
shouldTriggerto gate both cleanup and callback invocation, ensuring that semantically equal values (per customequals) skip unnecessary side-effects.
What this PR does
This PR introduces a new equals option to the watch and watchEffect APIs. This allows developers to provide a custom equality function to determine if the watcher's callback and its associated cleanup should be triggered.
Why we need it
Currently, onCleanup is invoked by the scheduler whenever the source reference changes, regardless of whether the semantic data is identical. In scenarios involving abortable side-effects (e.g., using AbortController.abort()), the cleanup runs before the callback comparison logic can intervene.
This makes it impossible to prevent unnecessary side-effect interruptions when using deep: true or watching objects that are frequently recreated but semantically equivalent. By providing a native equals option, we can short-circuit the effect lifecycle before the cleanup is even called.
Technical Implementation
@vue/reactivity: Modified the internal job in watch.ts to evaluate the options.equals function if provided. If the function returns true, the cleanup() and the callback are skipped.
@vue/runtime-core: Updated WatchOptions interface to expose the equals property to the end-user API.
Precedence: The equals option takes precedence over the default hasChanged check, ensuring full control over the trigger conditions.
Example Usage
Related Issue
Relates to vuejs/vue#13242 (Forward-port of the requested feature to Vue 3 Core)
Summary by CodeRabbit
New Features
Behavior
Tests
✏️ Tip: You can customize this high-level summary in your review settings.