-
Notifications
You must be signed in to change notification settings - Fork 5
Description
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':
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.