-
Notifications
You must be signed in to change notification settings - Fork 2k
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
0.8 array notification #1477
0.8 array notification #1477
Conversation
What's the motivation for this change? Is Array.observe too heavy-weight, or just not working right? |
@arthurevans Prior to this change, we used Array.observe (on Chrome) but on all other browsers overrode array instances with patched push/pop/etc. methods to be able to observe when changes occurred (remember, we're avoiding the observe-js polyfill overhead for Polymer proper). The patching approach was not only intrusive, but led to a hard-to-explain API w.r.t. making changes to structured data (if you want to set set
|
df3fe18
to
c5355a9
Compare
c5355a9
to
9860f47
Compare
LGTM |
Removes use of ArrayObserve.observe. Instead, users must use
this.push()
, etc. on elements to mutate arrays. Collection is also no longer observable; simply stands as a store of key->item and item->key mappings for arrays.Renames
getPathValue
andsetPathValue
toget
andset
.get
andset
can now take an array of path parts, to avoid needing to concat, e.g.set([items', 4, 'name'], 'bob);
, and also does automatic array index-to-key mapping when notifying path (4
in example would be replaced with the key foritems[4]
when notifying).Renames
_data
to__data__
to avoid collision with user data.