fix(runtime-vapor): refresh same v-for item refs#14788
Draft
edison1105 wants to merge 1 commit into
Draft
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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
|
edison1105
force-pushed
the
edison/fix/14787
branch
from
691df12 to
c287c97
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/runtime-vapor/src/apiCreateFor.ts`:
- Around line 168-182: The code uses a single boolean (shouldTriggerSameItems)
to decide whether to call triggerSameItemObjectRefs(newBlocks), which skips
triggering refs for individually reused blocks when length/order changes;
instead, change the logic in the unkeyed update loop to collect each reused
block whose update(...) returned false (e.g., push references from newBlocks[i]
/ oldBlocks[i] into a local array) and after all mount/unmount work call
triggerSameItemObjectRefs on that collected array; apply the same pattern
wherever triggerSameItemObjectRefs is invoked for reused items (the other reuse
sites / keyed paths referenced in the review) so each reused block that returned
false from update() is triggered individually regardless of
inserts/removes/moves.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ebd21eda-0504-4c35-8ed8-845e07ee3690
📒 Files selected for processing (2)
packages/runtime-vapor/__tests__/for.spec.tspackages/runtime-vapor/src/apiCreateFor.ts
Comment on lines
+168
to
+182
| let shouldTriggerSameItems = oldLength === newLength | ||
| for (let i = 0; i < commonLength; i++) { | ||
| update((newBlocks[i] = oldBlocks[i]), getItem(source, i)[0]) | ||
| if (update((newBlocks[i] = oldBlocks[i]), getItem(source, i)[0])) { | ||
| shouldTriggerSameItems = false | ||
| } | ||
| } | ||
| for (let i = oldLength; i < newLength; i++) { | ||
| mount(source, i) | ||
| } | ||
| for (let i = newLength; i < oldLength; i++) { | ||
| unmount(oldBlocks[i]) | ||
| } | ||
| if (shouldTriggerSameItems) { | ||
| triggerSameItemObjectRefs(newBlocks) | ||
| } |
There was a problem hiding this comment.
Track unchanged reused blocks individually, not with a single global flag.
This only retriggers item refs when the whole shared span keeps the same length/order. Mixed updates still miss stale rows. For example, with a shallowRef([a, b]), mutating a, then refreshing to [a, b, c] leaves a unchanged by identity, but Line 168 sets the gate false because the length changed, so triggerRef(aRef) never runs and its DOM stays stale. The same gap exists for replace/move cases in the keyed path once any other block changes.
A safer fix is to collect each reused block whose update() returned false and trigger only those after patching, regardless of whether other rows were inserted, removed, moved, or replaced.
Possible direction
- let shouldTriggerSameItems = oldLength === newLength
+ const sameItemBlocks: ForBlock[] = []
// whenever a block is reused
- if (update(block, newItem)) {
- shouldTriggerSameItems = false
- }
+ if (!update(block, newItem)) {
+ sameItemBlocks.push(block)
+ }
- if (shouldTriggerSameItems) {
- triggerSameItemObjectRefs(newBlocks)
- }
+ triggerSameItemObjectRefs(sameItemBlocks)You'd need to apply that consistently across the unkeyed path and every keyed reuse site.
Also applies to: 213-227, 371-373, 528-545
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/runtime-vapor/src/apiCreateFor.ts` around lines 168 - 182, The code
uses a single boolean (shouldTriggerSameItems) to decide whether to call
triggerSameItemObjectRefs(newBlocks), which skips triggering refs for
individually reused blocks when length/order changes; instead, change the logic
in the unkeyed update loop to collect each reused block whose update(...)
returned false (e.g., push references from newBlocks[i] / oldBlocks[i] into a
local array) and after all mount/unmount work call triggerSameItemObjectRefs on
that collected array; apply the same pattern wherever triggerSameItemObjectRefs
is invoked for reused items (the other reuse sites / keyed paths referenced in
the review) so each reused block that returned false from update() is triggered
individually regardless of inserts/removes/moves.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Close #14787
Summary by CodeRabbit
Bug Fixes
Tests