polymer: Array splicing and dom-repeat lead to wrong path in deep property observers
If you have an array that is modified using this.splice, this.push, and this.set from the Polymer library and have a dom-repeat iterating over the array, and then do these steps:
- Add some elements
- Remove some elements
- Change a property of any element physically AFTER the elements you removed
You will see that a deep observer of the array is given an incorrect path on the property change.
I believe this has to do with Polymer’s use of delete this.store[key] instead of, say, this.store.splice(key, 1) in the removeKey function. The WeakMap isn’t updated, and so future updates to the array are related back to mappings that no longer exist. Although I am not sure.
Example:
<dom-module id="test-element">
<template>
<template is="dom-repeat" items="{{tests}}">
<!--Nothing needs to happen in here-->
</template>
</template>
<script>
Polymer({
is: "test-element",
properties: {
"tests": {
type: Array,
notify: true,
value: []
}
},
observers: [
"_testsChanged(tests.*)"
],
"_testsChanged": function(changed) {
// With the dom-repeat, the wrong path is reported on subproperty change
// Without dom-repeat, it's fine
console.log("CHANGED:", changed, "PATH:", changed.path);
},
"add": function(value) {
this.push("tests", { "value": value });
},
"remove": function(index) {
this.splice("tests", index, 1);
},
"change": function(index, value) {
this.set("tests."+index+".value", value);
},
ready: function() {
this.runTest();
},
"runTest": function() {
this.add("a"); // [{ "value": "a" }]
this.add("b"); // [{ "value": "a" }, { "value": "b" }]
this.add("c"); // [{ "value": "a" }, { "value": "b" }. { "value": "c" }]
this.remove(0); // [{ "value": "b" }. { "value": "c" }]
this.remove(0); // [{ "value": "c" }]
this.change(0, "changed"); // [{ "value": "changed" }]
// Expected path: tests.0.value
// Actual path: tests.2.value
}
});
</script>
</dom-module>
Notice the order of operations. Adding some elements, removing any but the last, then changing the property of the last.
About this issue
- Original URL
- State: closed
- Created 9 years ago
- Comments: 17 (6 by maintainers)
Ruined full night on that misunderstanding of “#index” and “index” differences. Please change path naming conversion of array elements for something less confusing (i.e as @robdodson described in previous comment). Also, for some reason my array element do not have .base property. So in order to get key of array element used this fn:
Yeah it’s confusing.
persons.#8becomes a unique identifier for the item at array position2. The unique key let’s Polymer keep track of the item regardless of its position in the array. I almost wished that the unique key was more of a hash likecf23df2207d99aor something like that so it was really obvious that it had nothing to do with numeric position in an array.I’m not clear from your previous comment if we’ve answered your questions or if you’re still confused about something.
@arthurevans This distinction is NOT obvious in the documentation.
This is obviously a question only for the people who designed the system but… What use do we, as users, have for the static key?
I can only assume that the design decision had to do with runtime efficiency. One the one hand, they could have given us, and dom-repeat, the path to the elements in OUR array (not the static key), and left dom-repeat to convert to the path containing the static key. On the other hand, we have the current system where the users, and dom-repeat, are given the static key path and it’s the user’s job to get the path for their array.
Which leads to the question: which is more efficient? Which operation will run more, dom-repeat or user operations?
I believe the answer would be dom-repeat (seems the obvious answer). And so I can understand the choice that the designers made. They will save themselves some runtime by removing the need for dom-repeat to convert paths.
However, as a user of the library, it is very frustrating to have this thing called “path” not be a path in our array. This distinction is NOT obvious in the documentation.