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

Array observer delivers incorrect changeRecord.path index after deleting item in array #2007

Closed
borismus opened this issue Jul 1, 2015 · 4 comments
Assignees
Labels

Comments

@borismus
Copy link

borismus commented Jul 1, 2015

I register an array observer:

observers: [
    'sightChanged(sights.*)'
  ],

sightChanged is then called argument changeRecord = {path: 'sights.2.description', ...}. If I remove something from the sights array before index 2, the path 'sights.2.description' remains the same, even though it should now be decremented by one. This results in index out of bound problems:

screen shot 2015-07-01 at 12 27 14 pm

Seems like a bug to me. For the time being, is there a workaround to observing arrays?

@kevinpschaaf
Copy link
Member

Notifications for arrays use unique "keys" for rather than index, so what you are seeing is expected, although possibly unintuitive and not yet well documented. This is done to ensure path linkage between objects in arrays are maintained despite possible insertions or deletions in the array.

If you need the index, for now you can do this to workaround, using the Polymer.Collection abstraction that is used internally to manage keys for arrays:

var collection = Polymer.Collection.get(changeRecord.base);
var idx = changeRecord.base.indexOf(collection.getItem(key));

Where key is the number pulled from changeRecord.path.

We'll keep this issue open to consider how to improve the API.

@kevinpschaaf kevinpschaaf added the p3 label Jul 2, 2015
@kevinpschaaf kevinpschaaf self-assigned this Jul 2, 2015
@borismus
Copy link
Author

borismus commented Jul 2, 2015

Thanks. It might be nice also to have a helper method that would simplify parsing keys from pathes. Perhaps you could provide the key as changeRecord.key, or provide a Polymer.getModelByPath() method?

@kevinpschaaf
Copy link
Member

One thought is to disambiguate keys from indices in paths by using a token like # to denote a key, so that you could pass paths to set and get (currently set and get expect array indices, not keys, so the dichotomy is obviously bad and confusing). The path in the above example would then be sights.#2.description, and calling this.get('sights.#2') would return the correct base object and this.set('sights.#2.name', 'foo') would set a field on the base object.

Aside from needing to parse the base path (e.g. sights.#2.description -> sights.#2), would this addition to the API have helped solve your use case? Or did you actually need the index of the array (as opposed to the value at the index)?

@tomalec
Copy link
Contributor

tomalec commented Aug 13, 2015

Forgive me being paranoid, but how about escaping both # and even . used in current implementation? as .something and #1 are valid object keys

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants