Skip to content

Commit

Permalink
fix: better handling of recursive updates
Browse files Browse the repository at this point in the history
  • Loading branch information
LastLeaf committed Jun 24, 2024
1 parent ccdcc51 commit 53b8ce4
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 11 deletions.
9 changes: 9 additions & 0 deletions glass-easel/src/data_proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,7 @@ export class DataGroup<
combined: DataChange[]
count: number
} | null = null
private _$recUpdateLevel = 0

private _$generateInnerData(data: { [key: string]: DataValue }) {
const pureDataPattern = this._$pureDataPattern
Expand Down Expand Up @@ -990,7 +991,15 @@ export class DataGroup<
}

// tell template engine what changed
if (this._$recUpdateLevel > 0 && comp) {
triggerWarning(
`recursive update detected: a data update is applying during another data update for component "${comp.is}" - this may disorder node trees`,
comp,
)
}
this._$recUpdateLevel += 1
this._$updateListener?.(this.innerData || this.data, combinedChanges)
this._$recUpdateLevel -= 1

// trigger prop observers (to simulating legacy behaviors)
if (comp) {
Expand Down
15 changes: 11 additions & 4 deletions glass-easel/src/tmpl/proc_gen_wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ export class ProcGenWrapper {
(textContent: string | undefined) => {
const elem = childNodes[index] as TextNode
index += 1
if (!elem) return
if (slotElement) {
const tmplArgs = getTmplArgs(elem)
if (!tmplArgs.dynamicSlotNameMatched) {
Expand All @@ -456,6 +457,7 @@ export class ProcGenWrapper {
) => {
const elem = childNodes[index] as Element
index += 1
if (!elem) return
if (slotElement) {
const tmplArgs = getTmplArgs(elem)
if (dynamicSlotName! === (slot || '')) {
Expand Down Expand Up @@ -504,16 +506,17 @@ export class ProcGenWrapper {
(branchKey: number | string, branchFunc: DefineChildren) => {
const elem = childNodes[index] as Element
index += 1
const tmplArgs = getTmplArgs(elem)
if (tmplArgs.key === branchKey) {
if (!elem) return
const prevTmplArgs = getTmplArgs(elem)
if (prevTmplArgs.key === branchKey) {
this.handleChildrenUpdate(branchFunc, elem, slotElement, dynamicSlotName)
} else {
const newElem = this.shadowRoot.createVirtualNode('wx:if')
newElem.destroyBackendElementOnDetach()
Element.setInheritSlots(newElem)
if (slotElement) Element.setSlotElement(newElem, slotElement)
const tmplArgs = getTmplArgs(newElem)
tmplArgs.key = branchKey
prevTmplArgs.key = tmplArgs.key = branchKey
this.handleChildrenCreationAndInsert(branchFunc, newElem, slotElement, dynamicSlotName)
if (slotElement) parentNode.replaceChild(newElem, elem)
else parentNode.replaceChildAt(newElem, index - 1)
Expand Down Expand Up @@ -543,6 +546,7 @@ export class ProcGenWrapper {
) => {
const elem = childNodes[index] as Element
index += 1
if (!elem) return
const tmplArgs = getTmplArgs(elem)
const keyListManager = tmplArgs.keyList!
keyListManager.diff(
Expand Down Expand Up @@ -590,8 +594,9 @@ export class ProcGenWrapper {
index: string | number,
updatePathTree: UpdatePathTreeRoot,
indexChanged: boolean,
childNode: Element,
childNode: Element | undefined,
) => {
if (!childNode) return
this.handleChildrenUpdate(
(
isCreation,
Expand Down Expand Up @@ -629,6 +634,7 @@ export class ProcGenWrapper {
(slotName: string | undefined, slotValueInit?: (elem: Element) => void, slot?: string) => {
const elem = childNodes[index] as Element
index += 1
if (!elem) return
if (slotName !== undefined) {
Element.setSlotName(elem, dataValueToString(slotName))
}
Expand All @@ -643,6 +649,7 @@ export class ProcGenWrapper {
(children: DefineChildren, slot: string | undefined) => {
const elem = childNodes[index] as Element
index += 1
if (!elem) return
if (slot !== undefined) {
if (slotElement) {
const tmplArgs = getTmplArgs(elem)
Expand Down
26 changes: 19 additions & 7 deletions glass-easel/src/tmpl/range_list_diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export class RangeListManager {
index: string | number,
updatePathTree: UpdatePathTreeRoot,
indexChanged: boolean,
node: VirtualNode,
node: VirtualNode | undefined,
) => void,
) {
// generate new list for comparison
Expand Down Expand Up @@ -247,7 +247,13 @@ export class RangeListManager {
updatePathTree === true || updatePathTree === undefined
? updatePathTree
: (updatePathTree as { [key: string]: UpdatePathTreeNode })[index]
updateListItem(item, index, u, index !== oldIndex, elem.childNodes[i]! as VirtualNode)
updateListItem(
item,
index,
u,
index !== oldIndex,
elem.childNodes[i] as VirtualNode | undefined,
)
i += 1
}

Expand Down Expand Up @@ -331,7 +337,13 @@ export class RangeListManager {
updatePathTree === true || updatePathTree === undefined
? updatePathTree
: (updatePathTree as UpdatePathTreeNode[])[i]
updateListItem(item, index, u, index !== oldIndex, elem.childNodes[i]! as VirtualNode)
updateListItem(
item,
index,
u,
index !== oldIndex,
elem.childNodes[i] as VirtualNode | undefined,
)
i += 1
}
return
Expand Down Expand Up @@ -376,7 +388,7 @@ export class RangeListManager {
} else {
oldListOp[oldPos] = OpKind.ForwardMove
}
changedItems[i] = elem.childNodes[oldPos] as VirtualNode
changedItems[i] = elem.childNodes[oldPos] as VirtualNode | undefined
}

// visit the op list again and do the operations one-by-one
Expand Down Expand Up @@ -412,7 +424,7 @@ export class RangeListManager {

// insert or move items between two LCS items
while (opIndex < nextStable) {
const newItem = changedItems[opIndex]!
const newItem = changedItems[opIndex]
const oldPos = oldPosList[opIndex]!
if (oldPos === -1) {
const start = opIndex
Expand All @@ -428,7 +440,7 @@ export class RangeListManager {
)
realListDiff += count
} else {
elem.insertChildAt(newItem, nextStableOldPos + realListDiff)
if (newItem) elem.insertChildAt(newItem, nextStableOldPos + realListDiff)
const item = items[opIndex]!
const index = indexes === null ? opIndex : indexes[opIndex]!
const oldIndex = oldIndexes === null ? oldPos : oldIndexes[oldPos]!
Expand All @@ -455,7 +467,7 @@ export class RangeListManager {
updatePathTree === true || updatePathTree === undefined
? updatePathTree
: (updatePathTree as UpdatePathTreeNode[])[nextStable]
const node = elem.childNodes[nextStableOldPos + realListDiff]! as VirtualNode
const node = elem.childNodes[nextStableOldPos + realListDiff] as VirtualNode | undefined
updateListItem(item, index, u, index !== oldIndex, node)
}

Expand Down
34 changes: 34 additions & 0 deletions glass-easel/tests/core/data_update.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -891,4 +891,38 @@ describe('partial update', () => {
comp.setData(undefined)
expect(comp.data.a).toBe(456)
})

test('should warn recursive updates', () => {
const childCompDef = componentSpace
.define()
.property('a', String)
.observer('a', function () {
this.triggerEvent('b')
})
.registerComponent()
const compDef = componentSpace
.define()
.usingComponents({
child: childCompDef,
})
.template(
tmpl(`
<child wx:if="{{ cond }}" a="123" bind:b="ev" />
`),
)
.data(() => ({
cond: false,
}))
.methods({
ev() {
this.setData({ cond: true })
},
})
.registerComponent()
const comp = glassEasel.Component.createWithContext('root', compDef, domBackend)
glassEasel.Element.pretendAttached(comp)
execWithWarn(1, () => {
comp.setData({ cond: true })
})
})
})

0 comments on commit 53b8ce4

Please sign in to comment.