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

[BUGFIX beta] Prevent errors when using const (get arr 1). #16218

Merged
merged 1 commit into from
Feb 6, 2018

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
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
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