Skip to content
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.

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

can-view-live/list patcher does not properly check patch type or list capabilities #160

Open
rjgotten opened this issue Oct 1, 2021 · 0 comments
Assignees

Comments

@rjgotten
Copy link

rjgotten commented Oct 1, 2021

Currently the onPatches callback does not correctly check the type of received patches.
As you can see below, the code only checks for a type == "move" or 'anything else':

can-view-live/lib/list.js

Lines 148 to 168 in b0735a2

for (var i = 0, patchLen = sortedPatches.length; i < patchLen; i++) {
var patch = sortedPatches[i];
if (patch.type === "move") {
this.addToDomQueue(
this.move,
[patch.toIndex, patch.fromIndex]
);
} else {
if (patch.deleteCount) {
// Remove any items scheduled for deletion from the patch.
this.addToDomQueue(this.remove, [{
length: patch.deleteCount
}, patch.index]);
}
if (patch.insert && patch.insert.length) {
// Insert any new items at the index
this.addToDomQueue(this.add, [patch.insert, patch.index]);
}
}
}

'Anything else' can be problematic when you have a type which emits other type of patches in addition to list-like { type : "splice" } patches, like say... an observable variant of Set - which would emit { type : "values" } patches. These will be partially processed in a wrong way and might even involve exceptions being thrown. When really; they should just be ignored, because the type is incompatible.

The entire patcher also seems to not correctly use 'safe' means of iterating the lists using can-reflect.
It is possible to have a type that implements an iterator (and thus is deemed can.isListLike ) but which is NOT actually fully list-like and which does NOT implement index-based random access accessors.

@eddypjr eddypjr self-assigned this Oct 4, 2021
eddypjr pushed a commit that referenced this issue Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants