add assertions for tagless component event handlers #12503
Conversation
|
Yes, the custom events added by As discussed in #ember-dev in slack, I think the easiest/best way to do this would be via a small change to the var events = this._handledEvents = assign({}, get(this, 'events'), addedEvents);(or some better name than |
| this.tagName !== '' || !(() => { | ||
| var eventDispatcher = this.container.lookup('event_dispatcher:main'); | ||
| var application = this.container.lookup('application:main'); | ||
| var events = assign({}, eventDispatcher.events, application.customEvents); |
There was a problem hiding this comment.
I think this can be removed with that tweak to the event dispatcher we discussed.
|
I left one more minor nit-pick, but this looks great to me. @miguelcobain - Thank you for putting the time in and figuring this out! |
| `You can not define a function that handles DOM events in the \`${this}\` tagless component since it doesn't have any DOM element.`, | ||
| this.tagName !== '' || !(() => { | ||
| let eventDispatcher = this.container.lookup('event_dispatcher:main'); | ||
| let events = (eventDispatcher && eventDispatcher._finalEvents) || {}; |
There was a problem hiding this comment.
@rwjblue Like this? :)
I think a test may be a bit paranoic here. Are we testing && and ||?
|
Should be ready now! |
|
Restarted sauce labs tests. |
add assertions for tagless component event handlers
|
Thanks again @miguelcobain ! |
|
🎉 |
Should application instance custom events be considered as well?
Should I access it using
this.container.lookup('-application-instance:main')?Feel free to tag this PR if needed.
/cc @rwjblue @mixonic