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] Fix attrs in className and attribute bindings #14384

Merged
merged 1 commit into from
Sep 29, 2016

Conversation

chadhietala
Copy link
Contributor

No description provided.

@chadhietala
Copy link
Contributor Author

Fixes #14381


if (parts.length === 1) {
return referenceForKey(component, parts[0]);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the else here? is it supposed to fall through or assert?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want this to fall through. So given attrs.foo we want to delegate to referenceForKey, however if we shift off the value and we still have a path e.g. parts.length !== 1 (guranteed to never be 0) then we want to still create a path ref.

@@ -14,6 +14,17 @@ function referenceForKey(component, key) {
}

function referenceForParts(component, parts) {
let isAttrs = parts[0] === 'attrs';
Copy link
Contributor

Choose a reason for hiding this comment

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

is parts guaranteed to be length 1? is it very important that we never index outside of the array on get.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Upstream from this the check is done to make sure we have a path reference.

@krisselden krisselden merged commit 4a96d35 into emberjs:master Sep 29, 2016
@chadhietala chadhietala deleted the attrs-actions branch September 29, 2016 17:54
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.

2 participants