Skip to content

Conversation

@rwjblue
Copy link
Member

@rwjblue rwjblue commented Feb 6, 2018

Prior to this change, calling {{get arr 1}} would throw an error:

 pathReference.value(...).split is not a function

This commit fixes that, by only attempting to .split when the underlying value is not actually a string.

@rwjblue rwjblue force-pushed the fix-get-with-numeric branch from 11df44d to a65f533 Compare February 6, 2018 19:11
@rwjblue rwjblue requested review from krisselden and mmun February 6, 2018 19:30
@rwjblue
Copy link
Member Author

rwjblue commented Feb 6, 2018

FYI - The Ember.get codepath is essentially untouched with one small exception: I added an early return false guard to the isPath utility function (to avoid splitting when the argument is not a string).

return referenceFromParts(sourceReference, parts);
let value = pathReference.value();
if (typeof value === 'string' && value.indexOf('.') > -1) {
return referenceFromParts(sourceReference, value.split('.'));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I am working on a follow up PR (which will only target master) to make the referenceFromParts utility function accept a single value (instead of an array), that will remove this duplication (here and below)...

Prior to this change, calling `{{get arr 1}}` would throw an error:

```
 pathReference.value(...).split is not a function
```

This commit fixes that, by only attempting to `.split` when the
underlying value is not actually a string.

`Ember.get` / `Ember.set` are also updated to allow `get(obj, 2)`
(previously allowed only string keys).
@rwjblue rwjblue force-pushed the fix-get-with-numeric branch from a65f533 to 706166a Compare February 6, 2018 19:48
@rwjblue
Copy link
Member Author

rwjblue commented Feb 6, 2018

r+ from @krisselden in chat

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant