[BUGFIX lts] Ensure {{each-in}} can iterate over keys with periods #18296
Conversation
This PR refactors the iterator in `each-in` to use standard object get notation for getting values instead of `Ember.get`. This should work because: * All descriptors now install native getters * Object iterators use `Object.keys` to get the list of keys to iterate * `unknownProperty` is only ever triggered if a key is not `in` the object, which cannot be true if we just got the key using `Object.keys`. Thus `unknownProperty` will never be called for `each-in`. This will allow users to use arbitrary keys for their objects, and iterate over them with `each-in`. Keys will be tracked as if `get` were used.
|
Fixes #17529 |
delete-merged-branch
bot
deleted the
bugfix/ensure-each-in-can-iterate-over-arbitrary-keys
branch
| value = obj[key]; | ||
|
|
||
| // Add the tag of the returned value if it is an array, since arrays | ||
| // should always cause updates if they are consumed and then changed |
There was a problem hiding this comment.
Is that right? If you pass this to, e.g. #each, we handle this there already. If you just pass the value to, say, a component or a helper, I am not sure we need to invalidate those if the content of the array changes?
There was a problem hiding this comment.
I think the test you added should pass without consuming [].
There was a problem hiding this comment.
This is part of autotracking as a whole, it allows consumers to use arrays (both native and non-native) anywhere they consume a tracked value, and have mutations to that array be autotracked. In practice, this is like the native getters migration, it’ll mean users can access any array as if it were a normal, native array. They only need to use methods for updating the state of the array (we need native Proxy to allow them to update with native syntax).
Even when we do get to native Proxy though, this will allow us to omit a get handler in the Proxy, which should help us avoid perf issues with proxies
| let value: any; | ||
| let key = keys[i]; | ||
|
|
||
| value = obj[key]; |
There was a problem hiding this comment.
This breaks when obj is a proxy, is that okay?
In general, if the solution requires avoiding get semantics, I think that is not okay. However, maybe in this case it is fine since we already did Object.keys above so it won't trigger unknownProperty anyway.
There was a problem hiding this comment.
Correct, I started writing the code to do unknownProperty and realized that by definition, it could never run, which is why I omitted it.
This was fixed in emberjs/ember.js#18296 but I can’t bear to update Ember at the moment. This is untested code 😳
This PR refactors the iterator in
each-into use standard object getnotation for getting values instead of
Ember.get. This should workbecause:
Object.keysto get the list of keys to iterateunknownPropertyis only ever triggered if a key is notintheobject, which cannot be true if we just got the key using
Object.keys. ThusunknownPropertywill never be called foreach-in.This will allow users to use arbitrary keys for their objects, and
iterate over them with
each-in. Keys will be tracked as ifgetwereused.