Skip to content

fix(runtime-vapor): refresh same v-for item refs #14788

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.

Sign up for GitHub

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

Draft
edison1105 wants to merge 1 commit into
base: minor
Choose a base branch
from
Draft

fix(runtime-vapor): refresh same v-for item refs #14788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 96 additions & 3 deletions packages/runtime-vapor/__tests__/for.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,20 +426,20 @@ describe('createFor', () => {
'<li>0. 1</li><li>1. 2</li><li>2. 3</li><li>3. 4</li><!--for-->',
)

// change deep value should not update
// change deep value and refresh source
list.value[0].name = 'a'
setList()
await nextTick()
expect(host.innerHTML).toBe(
'<li>0. 1</li><li>1. 2</li><li>2. 3</li><li>3. 4</li><!--for-->',
'<li>0. a</li><li>1. 2</li><li>2. 3</li><li>3. 4</li><!--for-->',
)

// remove
list.value.splice(1, 1)
setList()
await nextTick()
expect(host.innerHTML).toBe(
'<li>0. 1</li><li>1. 3</li><li>2. 4</li><!--for-->',
'<li>0. a</li><li>1. 3</li><li>2. 4</li><!--for-->',
)

// clear
Expand All @@ -448,6 +448,82 @@ describe('createFor', () => {
expect(host.innerHTML).toBe('<!--for-->')
})

test('should update same item references when source is refreshed', async () => {
let rawList = [{ number: 0 }, { number: 1 }]
const list = shallowRef(rawList)

const { host } = define(() => {
const n1 = createFor(
() => list.value,
item => {
const span = document.createElement('li')
renderEffect(() => {
span.textContent = JSON.stringify(item.value)
})
return span
},
(_item, key) => `${key}-test`,
)
return n1
}).render()

expect(host.innerHTML).toBe(
'<li>{"number":0}</li><li>{"number":1}</li><!--for-->',
)

rawList[0].number = 2
list.value = rawList.slice()
await nextTick()
expect(host.innerHTML).toBe(
'<li>{"number":2}</li><li>{"number":1}</li><!--for-->',
)

list.value[0].number = 3
triggerRef(list)
await nextTick()
expect(host.innerHTML).toBe(
'<li>{"number":3}</li><li>{"number":1}</li><!--for-->',
)

rawList = [{ number: 0 }, { number: 1 }]
list.value = rawList
await nextTick()
expect(host.innerHTML).toBe(
'<li>{"number":0}</li><li>{"number":1}</li><!--for-->',
)
})

test('should update same reactive item references when source is replaced', async () => {
const rawList = [{ number: 0 }, { number: 1 }]
const list = ref(rawList)

const { host } = define(() => {
const n1 = createFor(
() => list.value,
item => {
const span = document.createElement('li')
renderEffect(() => {
span.textContent = JSON.stringify(item.value)
})
return span
},
(_item, key) => `${key}-test`,
)
return n1
}).render()

expect(host.innerHTML).toBe(
'<li>{"number":0}</li><li>{"number":1}</li><!--for-->',
)

rawList[0].number = 2
list.value = rawList.slice()
await nextTick()
expect(host.innerHTML).toBe(
'<li>{"number":2}</li><li>{"number":1}</li><!--for-->',
)
})

test('should optimize call frequency during list operations', async () => {
let sourceCalledTimes = 0
let renderCalledTimes = 0
Expand Down Expand Up @@ -519,6 +595,14 @@ describe('createFor', () => {
await nextTick()
expectCalledTimesToBe('Update every 10th row', 0, 0, length() / 10, 0)

// Replace a row with the same key
list.value[0] = {
id: list.value[0].id,
label: list.value[0].label + 10000,
}
await nextTick()
expectCalledTimesToBe('Replace a row with the same key', 1, 0, 1, 0)

// Append rows
list.value.push(...createItems(100))
await nextTick()
Expand Down Expand Up @@ -638,6 +722,15 @@ describe('createFor', () => {
await nextTick()
expectCalledTimesToBe('Update every 10th row', 0, 0, length() / 10, 0)

// Replace a row with the same key
list.value[0] = {
id: list.value[0].id,
label: shallowRef(list.value[0].label.value + 10000),
}
triggerRef(list)
await nextTick()
expectCalledTimesToBe('Replace a row with the same key', 1, 0, 1, 0)

// Append rows
list.value.push(...createItems(100))
triggerRef(list)
Expand Down
30 changes: 26 additions & 4 deletions packages/runtime-vapor/src/apiCreateFor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
shallowRef,
toReactive,
toReadonly,
triggerRef,
watch,
} from '@vue/reactivity'
import { isArray, isObject, isString } from '@vue/shared'
Expand Down Expand Up @@ -164,15 +165,21 @@ export const createFor = (
} else if (!getKey) {
// unkeyed fast path
const commonLength = Math.min(newLength, oldLength)
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)
}
Comment on lines +168 to +182
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

} else {
if (__DEV__) {
const keyToIndexMap: Map<any, number> = new Map()
Expand Down Expand Up @@ -203,17 +210,21 @@ export const createFor = (
let endOffset = 0
let queuedBlocksLength = 0
let oldKeyIndexPairsLength = 0
let shouldTriggerSameItems = oldLength === newLength

while (endOffset < commonLength) {
const index = newLength - endOffset - 1
const item = getItem(source, index)
const key = getKey(...item)
const existingBlock = oldBlocks[oldLength - endOffset - 1]
if (existingBlock.key !== key) break
update(existingBlock, ...item)
if (update(existingBlock, ...item)) {
shouldTriggerSameItems = false
}
newBlocks[index] = existingBlock
endOffset++
}
if (endOffset !== commonLength) shouldTriggerSameItems = false

const e1 = commonLength - endOffset
const e2 = oldLength - endOffset
Expand Down Expand Up @@ -357,9 +368,11 @@ export const createFor = (
block.prevAnchor = block.next = block.prev = undefined
}
}
if (shouldTriggerSameItems) {
triggerSameItemObjectRefs(newBlocks)
}
}
}

frag.nodes = [(oldBlocks = newBlocks)]
if (parentAnchor) frag.nodes.push(parentAnchor)

Expand Down Expand Up @@ -512,7 +525,8 @@ export const createFor = (
newKey?: any,
newIndex?: any,
) => {
if (newItem !== itemRef.value) {
const itemChanged = newItem !== itemRef.value
if (itemChanged) {
itemRef.value = newItem
}
if (keyRef && newKey !== undefined && newKey !== keyRef.value) {
Expand All @@ -521,6 +535,14 @@ export const createFor = (
if (indexRef && newIndex !== undefined && newIndex !== indexRef.value) {
indexRef.value = newIndex
}
return itemChanged
}

function triggerSameItemObjectRefs(blocks: ForBlock[]): void {
for (let i = 0; i < blocks.length; i++) {
const itemRef = blocks[i].itemRef
if (isObject(itemRef.value)) triggerRef(itemRef)
}
}

const unmount = (block: ForBlock, doRemove = true, doDeregister = true) => {
Expand Down
Loading