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

[Backport] Add exception for didRemoveListener so evented proxy objects can #16496

Merged
merged 1 commit into from
Apr 11, 2018

Conversation

rondale-sc
Copy link
Contributor

@rondale-sc rondale-sc commented Apr 11, 2018

When off is called on an Evented proxyish object it checks to see if
didRemoveListener is a function (so that it can later call it). This
check triggers the assertion for proxy-ish objects to use get rather
than installed es5-getter so we add a special case to allow it.

Prior to this change when you called off on the proxyish object it
would trigger an assertion telling you to call
eventedProxyObj.get('didRemoveListener') rather than eventedProxyObj.didRemoveListener`.

A few notes about this test:

  • the unknownProperty is how we let ember know this is a proxy
  • the unknownProperty function must return a value other than undefined
  • Evented Mixin must be mixed in

function

When `off` is called on an Evented proxyish object it checks to see if
didRemoveListener is a function (so that it can later call it).  This
check triggers the assertion for proxy-ish objects to use `get` rather
than installed es5-getter so we add a special case to allow it.

Prior to this change when you called `off` on the proxyish object it
would trigger an assertion telling you to call
`eventedProxyObj.get('didRemoveListener') rather than
`eventedProxyObj.didRemoveListener`.

A few notes about this test:

- the unknownProperty is how we let ember know this is a proxy
- the unknownProperty function must return a value other than undefined
- Evented Mixin must be mixed in
@rwjblue rwjblue merged commit 50db6dc into emberjs:release Apr 11, 2018
@rwjblue
Copy link
Member

rwjblue commented Apr 11, 2018

Sorry for missing this, thanks for fixing!

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