-
-
Notifications
You must be signed in to change notification settings - Fork 4.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
[BUGFIX] Remove vestigial path normalization in chains #12929
Conversation
deprecate( | ||
`Connecting an Ember.Binding to a global path such as '${this._from}' is deprecated. Please use a service instead.`, | ||
false, | ||
{ id: 'ember-metal.global-bindings', until: '3.0.0' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please make an entry in the deprecation guide and add a url
property here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's discuss whether we want to deprecate all of Ember.Binding first. I think we should.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On that note I am going to remove the deprecation from this particular PR.
73b6aa5
to
59fa972
Compare
Kris pointed out a couple other things that should be cleaned up before merging:
And we should do some performance testing to see how much of a win it is. |
🔥 🎉 |
Removes support for paths containing globals and "this". These were intended to be removed at the time when globals support was removeded from Ember.get/set and from inside of temlates, e.g. ```hbs {{SomeGlobal.foo}} ```
59fa972
to
419fc72
Compare
Thanks @mmun! |
[BUGFIX] Remove vestigial path normalization in chains
@@ -219,20 +232,20 @@ Binding.prototype = { | |||
|
|||
/* Called when the from side changes. */ | |||
fromDidChange(target) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this arg doesn't seem one used anymore?
This should be documented as breaking but since the behavior made no sense and didn't align with templates I doubt this will bite anyone |
This fixes a bug that occurs when chaining through a property that starts with a capital letter.
Here is a JSBin with a demonstration of the bug.