fix(compat): watch arrays as deep: 1 #13377
Conversation
# Conflicts: # packages/reactivity/src/watch.ts
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughCompatibility logic for array watching was added and refactored across the reactivity and runtime-core modules. The watch callback triggering conditions were extended to support legacy array semantics under a compatibility flag. The watcher getter logic was centralized and reused, and the test suite for the Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant Watcher
participant CompatGetter
participant Callback
Component->>Watcher: Register watch on array property (compat mode)
Watcher->>CompatGetter: Get value (array)
CompatGetter->>Watcher: Return deeply traversed array (depth 1)
Watcher->>Callback: Invoke callback if compatWatchArray flag and array detected
Poem
✨ 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 |
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/compiler-vapor
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/runtime-vapor
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
Size ReportBundles
Usages
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/runtime-core/src/apiWatch.ts (1)
289-299: Address static analysis suggestion for optional chaining.The compatibility logic is correct, but there's an opportunity to improve code quality by using optional chaining as suggested by the static analysis tool.
Apply this diff to use optional chaining:
- const deep = options && options.deep + const deep = options?.deep🧰 Tools
🪛 Biome (1.9.4)
[error] 294-294: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/reactivity/src/watch.ts(1 hunks)packages/runtime-core/src/apiWatch.ts(5 hunks)packages/runtime-core/src/componentOptions.ts(5 hunks)packages/vue-compat/__tests__/misc.spec.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/reactivity/src/watch.ts (1)
packages/shared/src/general.ts (2)
hasChanged(143-144)isArray(39-39)
packages/runtime-core/src/componentOptions.ts (3)
packages/runtime-core/src/index.ts (1)
DeprecationTypes(479-481)packages/shared/src/general.ts (3)
isObject(53-54)isArray(39-39)isFunction(49-50)packages/runtime-core/src/apiWatch.ts (1)
createCompatWatchGetter(252-266)
🪛 Biome (1.9.4)
packages/runtime-core/src/apiWatch.ts
[error] 294-294: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
🔇 Additional comments (5)
packages/reactivity/src/watch.ts (1)
248-249: LGTM: Correctly implements array watching compatibility.The additional condition properly bypasses normal change detection for arrays under compatibility mode, ensuring Vue 2-like behavior where array mutations trigger watchers even when the array reference hasn't changed.
packages/runtime-core/src/apiWatch.ts (1)
252-266: LGTM: Well-designed compatibility wrapper function.The
createCompatWatchGetterfunction correctly wraps the base getter and conditionally traverses arrays with depth 1 when theWATCH_ARRAYcompatibility flag is enabled. This approach is clean and reusable.packages/vue-compat/__tests__/misc.spec.ts (1)
50-421: Excellent comprehensive test coverage.The expanded test suite thoroughly covers the
WATCH_ARRAYcompatibility feature with well-structured scenarios for both thewatchoption and$watch()method. The tests appropriately verify:
- Basic array watching behavior and deprecation warnings
- Dynamic depth behavior when switching between objects and arrays
- Proper handling of the
deep: trueoption- Component-level
compatConfigoverrides- Various option combinations (immediate, etc.)
- Function getter behavior
This comprehensive coverage will help prevent regressions and ensure the compatibility feature works as expected.
packages/runtime-core/src/componentOptions.ts (2)
1-35: Clean import organization with proper type imports.The import changes properly use
import typefor type-only imports and remove unused imports (traverse,getCurrentScope). The addition ofcreateCompatWatchGetteraligns with the new compatibility approach.
857-866: Improved compatibility logic using centralized approach.The refactored compatibility logic is cleaner and more maintainable than the previous approach. Instead of forcing
deep: true, it now:
- Properly checks if
deepis already explicitly set- Uses the
compatWatchArrayflag to signal compatibility mode- Leverages the reusable
createCompatWatchGetterwrapperThis approach provides better separation of concerns and consistency across the codebase.
edison1105
added
version: minor
🔨 p3-minor-bug
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
|
@skirtles-code |
# Conflicts: # packages/reactivity/src/watch.ts # packages/runtime-core/src/apiWatch.ts # packages/runtime-core/src/componentOptions.ts
|
Updated to use I had to add some extra lines to copy |
4 participants
This PR builds on the work done in #12236 to try to make the compat build more accurately emulate Vue 2.
Watching arrays in Vue 2 behaved like
deep: 1, rather thandeep: true, so mutating nested objects should not trigger the watcher. Support for a numeric depth was added in 3.5 and wasn't available when the compat build was first implemented.But there are several other inconsistencies in the compat build. Some of these seem to have been introduced by #9927, and worked correctly prior to 3.5:
nullwhile data is loading.this.$watch(), not just thewatchoption.deep: truehas been set.There were relatively few tests for
WATCH_ARRAY, so it's understandable that these regressions were introduced. I've added a lot of extra tests to try to avoid future problems.I've introduced a
compatWatchArrayflag to the internal watcher options as a way to tell@vue/reactivitythat we want the special handling of arrays. This only impacts the equality check, which needs to behave like a deep watcher only if the current value is an array. I didn't want to add this flag to the public types, so there's a bit ofas anyhackery to access it.Summary by CodeRabbit
New Features
Bug Fixes
Tests