-
-
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
Enable global event dispatcher listeners to be lazily created. #17911
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,12 @@ | ||
import { Compiler } from '@glimmer/interfaces'; | ||
import RuntimeResolver from './resolver'; | ||
import { getOwner } from '@ember/-internals/owner'; | ||
|
||
// factory for DI | ||
export default { | ||
create(): Compiler { | ||
return new RuntimeResolver().compiler; | ||
create(props: any): Compiler { | ||
let owner = getOwner(props); | ||
|
||
return new RuntimeResolver(owner).compiler; | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -127,6 +127,9 @@ export default EmberObject.extend({ | |
); | ||
|
||
this._eventHandlers = Object.create(null); | ||
this._finalEventNameMapping = null; | ||
this._sanitizedRootElement = null; | ||
this._lazyEvents = new Map(); | ||
}, | ||
|
||
/** | ||
|
@@ -142,7 +145,8 @@ export default EmberObject.extend({ | |
@param addedEvents {Object} | ||
*/ | ||
setup(addedEvents, _rootElement) { | ||
let events = (this._finalEvents = assign({}, get(this, 'events'), addedEvents)); | ||
let events = (this._finalEventNameMapping = assign({}, get(this, 'events'), addedEvents)); | ||
let lazyEvents = this._lazyEvents; | ||
|
||
if (_rootElement !== undefined && _rootElement !== null) { | ||
set(this, 'rootElement', _rootElement); | ||
|
@@ -216,9 +220,13 @@ export default EmberObject.extend({ | |
} | ||
} | ||
|
||
// save off the final sanitized root element (for usage in setupHandler) | ||
this._sanitizedRootElement = rootElement; | ||
|
||
// setup event listeners for the non-lazily setup events | ||
for (let event in events) { | ||
if (events.hasOwnProperty(event)) { | ||
this.setupHandler(rootElement, event, events[event]); | ||
lazyEvents.set(event, events[event]); | ||
} | ||
} | ||
}, | ||
|
@@ -234,12 +242,16 @@ export default EmberObject.extend({ | |
@private | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change makes this method less "private" I think, at least "private" in the OOP-sense of being able to call it from outside (remains "private" in the semver-sense that it shouldn't be used in user-land). At least when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ya, unfortunately |
||
@method setupHandler | ||
@param {Element} rootElement | ||
@param {String} event the browser-originated event to listen to | ||
@param {String} event the name of the event in the browser | ||
@param {String} eventName the name of the method to call on the view | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The order of arguments has changed, and should be reflected in the DocBlock. |
||
*/ | ||
setupHandler(rootElement, event, eventName) { | ||
if (eventName === null) { | ||
return; | ||
setupHandler( | ||
event, | ||
eventName = this._finalEventNameMapping[event], | ||
rootElement = this._sanitizedRootElement | ||
) { | ||
if (eventName === null || !this._lazyEvents.has(event)) { | ||
return; // nothing to do | ||
} | ||
|
||
if (!JQUERY_INTEGRATION || jQueryDisabled) { | ||
|
@@ -417,6 +429,8 @@ export default EmberObject.extend({ | |
} | ||
}); | ||
} | ||
|
||
this._lazyEvents.delete(event); | ||
}, | ||
|
||
destroy() { | ||
|
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 also needs to be "semi-public", in terms of
@ember/jquery
, see the other comment...