Skip to content

fix(frontend): handle multi-item array updates in updateYTypeFromObject#4265

Open
carloea2 wants to merge 1 commit intoapache:mainfrom
carloea2:fix/share-editing-array-lcs
Open

fix(frontend): handle multi-item array updates in updateYTypeFromObject#4265
carloea2 wants to merge 1 commit intoapache:mainfrom
carloea2:fix/share-editing-array-lcs

Conversation

@carloea2
Copy link
Contributor

@carloea2 carloea2 commented Mar 7, 2026

What changes were proposed in this PR?

This PR fixes the pending TODO from Array branch in updateYTypeFromObject(...).

} else if (newObjType === "Array") {
// TODO: Fix this
const oldYObjAsYArray = oldYObj as unknown as Y.Array<any>;
const newObjAsArr = newObj as any[];
const newArrLen = newObjAsArr.length;
const oldObjAsArr = oldYObjAsYArray.toJSON();
const oldArrLen = oldObjAsArr.length;
// TODO: in-place update, assuming only one update at a time can happen.
if (newArrLen < oldArrLen) {
let i = 0;
for (i; i < newArrLen; i++) {
if (!_.isEqual(oldObjAsArr[i], newObjAsArr[i])) break;
}
oldYObjAsYArray.delete(i);
} else if (newArrLen > oldArrLen) {
let i = 0;
for (i; i < newArrLen; i++) {
if (!_.isEqual(oldObjAsArr[i], newObjAsArr[i])) break;
}
oldYObjAsYArray.insert(i, [createYTypeFromObject(newObjAsArr[i])]);
} else {
for (let i = 0; i < newArrLen; i++) {
if (!_.isEqual(oldObjAsArr[i], newObjAsArr[i])) {
if (!updateYTypeFromObject(oldYObjAsYArray.get(i), newObjAsArr[i])) {
if (newObjAsArr[i] !== undefined) {
oldYObjAsYArray.delete(i, 1);
const res = createYTypeFromObject(newObjAsArr[i]);
if (res === undefined) oldYObjAsYArray.insert(i, [null]);
else oldYObjAsYArray.insert(i, [res]);
}
}
}
}
}
} else if (newObjType === "Object") {

Previously, the implementation assumed only one array update happened at a time and only handled the first mismatch with a single insert or delete. This caused incorrect behavior for multi-item insertions, multi-item deletions, and reorder cases.

This PR replaces that logic with an Longest Common Subsequence based array diff so the final Y.Array matches the target array more reliably while still attempting in-place updates where possible.

Any related issues, documentation, discussions?

Closes #4264

How was this PR tested?

Manually tested using:

[1, 2, 3, 4, 5] -> [1, 5]
[1, 2, 3, 4, 5] -> [7, 5, 8, 1 6]
[1, 2] -> [1, 3, 4, 2]
["x", "y", "z"] -> ["z", "x", "y"]

Verified that the final Y.Array content matches the target array.

Was this PR authored or co-authored using generative AI tooling?

Co-generated with GPT

@github-actions github-actions bot added fix frontend Changes related to the frontend GUI labels Mar 7, 2026
@carloea2
Copy link
Contributor Author

carloea2 commented Mar 7, 2026

@chenlica @Xiao-zhen-Liu
The required PR before UiParameter PR.

Thanks

@carloea2
Copy link
Contributor Author

carloea2 commented Mar 7, 2026

The new version has O(oldArrLen * newArrLen) time and space complexity, compared with the previous linear O(newArrLen) behavior.

@chenlica chenlica requested a review from Xiao-zhen-Liu March 7, 2026 16:03
@chenlica
Copy link
Contributor

chenlica commented Mar 7, 2026

@Xiao-zhen-Liu Please review it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

updateYTypeFromObject array update logic fails for multi-item insert/delete and reorder cases

2 participants