-
-
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
[CLEANUP beta] Removed deprecated currentWhen of LinkComponent #11793
[CLEANUP beta] Removed deprecated currentWhen of LinkComponent #11793
Conversation
currentWhen = attrs.currentWhen; | ||
} | ||
|
||
if (currentWhen) { |
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.
Note: I removed this lines, I haven't see the need to duplicate the same attribute under two different keys, both in this.attrs
and under the object itself (and with different casing).
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 is a breaking change without deprecation as it exists here.
Consider the following usage:
{{link-to 'stuff' 'stuff' current-when='foo'}}
In the above invocation, attrs['current-when']
was previously setting currentWhen
so that you could call this.get('currentWhen')
(instead of this.get('current-when')
). We could add a deprecatingAlias
from currentWhen
to current-when
if we need to...
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.
I considered that, but since the currentWhen
attribute was deprecated explicitly set and was marked as private in the docs I wasn't sure if I had to go through the intermediate step. Do I?
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.
ping on this?
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.
@cibernox even changing private APIs we like to do a check to see if there is usage in the wild.
Unfortunately it seems like there is plenty: https://github.com/search?q=currentWhen&type=Code&utf8=%E2%9C%93 so we should add a corresponding deprecation in 1.13 :-/
|
e19e038
to
1ace9ce
Compare
Use `current-when` instead
1ace9ce
to
83ff1c7
Compare
Rebased after deprecated code got one of those fancy deprecation ids. |
@rwjblue please confirm that the deprecation the removes should have been |
I kinda thought we'd keep this around until 3.0.0 (hence the Gonna close this for now. Others please feel free to argue with me 😺 |
Unrelated, but any chance we could make |
@btecu it should be public afaik. If it is marked otherwise a PR is definitely welcome. |
Use
current-when
instead