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] Allow call, apply to trigger _super wrapping. #12359

Merged
merged 1 commit into from
Sep 18, 2015

Conversation

mixonic
Copy link
Sponsor Member

@mixonic mixonic commented Sep 17, 2015

So, the new _super logic coming in 2.1 appears to have a slightly different behavior than that of 2.0 regarding making _super available to CPs. Because the getter function of this macro does not contain the string this._super, the super is not provided (and ROOT is incorrectly used instead).

This change wraps methods using call and apply in addition to those using _super in case super is called.

@mixonic mixonic changed the title Add a failing test for super in CP macro [BUGFIX beta] Allow call, apply to trigger _super wrapping. Sep 18, 2015
@mixonic
Copy link
Sponsor Member Author

mixonic commented Sep 18, 2015

Now updated to be a PR with [BUGFIX beta], per discussion with @krisselden

@mixonic mixonic force-pushed the the-bastard branch 2 times, most recently from b113b23 to 57b9577 Compare September 18, 2015 03:31
For CP macros (and other possible cases), `call` or `apply` may be used
to redirect what seems like the "method" of a property. These cases
should be wrapped in case they use `_super`, much as the explicit check
for `_super` usages causes the method to be wrapped.
rwjblue added a commit that referenced this pull request Sep 18, 2015
[BUGFIX beta] Allow call, apply to trigger `_super` wrapping.
@rwjblue rwjblue merged commit 85f2cd4 into emberjs:master Sep 18, 2015
@rwjblue rwjblue deleted the the-bastard branch September 18, 2015 14:31
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