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

Enable global event dispatcher listeners to be lazily created. #19227

Merged
merged 15 commits into from
May 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 50 additions & 3 deletions packages/@ember/-internals/glimmer/lib/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ import {
ChildViewsSupport,
ClassNamesSupport,
CoreView,
EventDispatcher,
getViewElement,
ViewMixin,
ViewStateSupport,
} from '@ember/-internals/views';
import { EMBER_MODERNIZED_BUILT_IN_COMPONENTS } from '@ember/canary-features';
import { assert, deprecate } from '@ember/debug';
import { DEBUG } from '@glimmer/env';
import { Environment } from '@glimmer/interfaces';
import { setInternalComponentManager } from '@glimmer/manager';
import { isUpdatableRef, updateRef } from '@glimmer/reference';
import { normalizeProperty } from '@glimmer/runtime';
Expand All @@ -26,6 +28,11 @@ import {
IS_DISPATCHING_ATTRS,
} from './component-managers/curly';

// Keep track of which component classes have already been processed for lazy event setup.
// Using a WeakSet would be more appropriate here, but this can only be used when IE11 support is dropped.
// Thus the workaround using a WeakMap<object, true>
let lazyEventsProcessed = new WeakMap<EventDispatcher, WeakMap<object, true>>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, we have a WeakSet polyfill thingy that we use (which lets us treat it like a WeakSet, but on IE11 is implemented as a WeakMap).

import { _WeakSet as WeakSet } from '@glimmer/util';

https://github.com/glimmerjs/glimmer-vm/blob/master/packages/@glimmer/util/lib/weak-set.ts


/**
@module @ember/component
*/
Expand Down Expand Up @@ -660,10 +667,31 @@ const Component = CoreView.extend(
this[DIRTY_TAG] = createTag();
this[BOUNDS] = null;

if (DEBUG && this.renderer._isInteractive && this.tagName === '') {
let eventDispatcher = this._dispatcher;
if (eventDispatcher) {
let lazyEventsProcessedForComponentClass = lazyEventsProcessed.get(eventDispatcher);
if (!lazyEventsProcessedForComponentClass) {
lazyEventsProcessedForComponentClass = new WeakMap<object, true>();
lazyEventsProcessed.set(eventDispatcher, lazyEventsProcessedForComponentClass);
}

let proto = Object.getPrototypeOf(this);
if (!lazyEventsProcessedForComponentClass.has(proto)) {
let lazyEvents = eventDispatcher.lazyEvents;

lazyEvents.forEach((mappedEventName: string, event: string) => {
if (mappedEventName !== null && typeof this[mappedEventName] === 'function') {
eventDispatcher.setupHandlerForBrowserEvent(event);
}
});

lazyEventsProcessedForComponentClass.set(proto, true);
}
}

if (DEBUG && eventDispatcher && this.renderer._isInteractive && this.tagName === '') {
let eventNames = [];
let eventDispatcher = getOwner(this).lookup<any | undefined>('event_dispatcher:main');
let events = (eventDispatcher && eventDispatcher._finalEvents) || {};
let events = eventDispatcher.finalEventNameMapping;

// tslint:disable-next-line:forin
for (let key in events) {
Expand Down Expand Up @@ -722,6 +750,25 @@ const Component = CoreView.extend(
);
},

get _dispatcher(): EventDispatcher | null {
if (this.__dispatcher === undefined) {
let owner = getOwner(this);
if (owner.lookup<Environment>('-environment:main')!.isInteractive) {
this.__dispatcher = owner.lookup<EventDispatcher>('event_dispatcher:main');
} else {
// In FastBoot we have no EventDispatcher. Set to null to not try again to look it up.
this.__dispatcher = null;
}
}

return this.__dispatcher;
},

on(eventName: string) {
this._dispatcher?.setupHandlerForEmberEvent(eventName);
return this._super(...arguments);
},

rerender() {
dirtyTag(this[DIRTY_TAG]);
this._super();
Expand Down
21 changes: 17 additions & 4 deletions packages/@ember/-internals/glimmer/lib/modifiers/action.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Owner } from '@ember/-internals/owner';
import { uuid } from '@ember/-internals/utils';
import { ActionManager, isSimpleClick } from '@ember/-internals/views';
import { ActionManager, EventDispatcher, isSimpleClick } from '@ember/-internals/views';
import { assert, deprecate } from '@ember/debug';
import { flaggedInstrument } from '@ember/instrumentation';
import { join } from '@ember/runloop';
Expand Down Expand Up @@ -65,6 +65,7 @@ export let ActionHelper = {

export class ActionState {
public element: SimpleElement;
public owner: Owner;
public actionId: number;
public actionName: any;
public actionArgs: any;
Expand All @@ -76,12 +77,14 @@ export class ActionState {

constructor(
element: SimpleElement,
owner: Owner,
actionId: number,
actionArgs: any[],
namedArgs: CapturedNamedArguments,
positionalArgs: CapturedPositionalArguments
) {
this.element = element;
this.owner = owner;
this.actionId = actionId;
this.actionArgs = actionArgs;
this.namedArgs = namedArgs;
Expand Down Expand Up @@ -200,7 +203,7 @@ export class ActionState {

class ActionModifierManager implements InternalModifierManager<ActionState, object> {
create(
_owner: Owner,
owner: Owner,
element: SimpleElement,
_state: object,
{ named, positional }: CapturedArguments
Expand All @@ -213,7 +216,7 @@ class ActionModifierManager implements InternalModifierManager<ActionState, obje
}

let actionId = uuid();
let actionState = new ActionState(element, actionId, actionArgs, named, positional);
let actionState = new ActionState(element, owner, actionId, actionArgs, named, positional);

deprecate(
`Using the \`{{action}}\` modifier with \`${actionState.eventName}\` events has been deprecated.`,
Expand Down Expand Up @@ -277,6 +280,7 @@ class ActionModifierManager implements InternalModifierManager<ActionState, obje
actionState.actionName = actionName;
actionState.implicitTarget = implicitTarget;

this.ensureEventSetup(actionState);
ActionHelper.registerAction(actionState);

element.setAttribute('data-ember-action', '');
Expand All @@ -291,7 +295,16 @@ class ActionModifierManager implements InternalModifierManager<ActionState, obje
actionState.actionName = valueForRef(actionNameRef);
}

actionState.eventName = actionState.getEventName();
let newEventName = actionState.getEventName();
if (newEventName !== actionState.eventName) {
this.ensureEventSetup(actionState);
actionState.eventName = actionState.getEventName();
}
}

ensureEventSetup(actionState: ActionState): void {
let dispatcher = actionState.owner.lookup<EventDispatcher>('event_dispatcher:main');
dispatcher?.setupHandlerForEmberEvent(actionState.eventName);
}

getTag(actionState: ActionState): UpdatableTag {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { RenderingTestCase, moduleFor, runTask } from 'internal-test-helpers';

import { Component } from '../utils/helpers';
import { _getCurrentRunLoop, run } from '@ember/runloop';
import { _getCurrentRunLoop } from '@ember/runloop';
import {
subscribe as instrumentationSubscribe,
reset as instrumentationReset,
Expand All @@ -20,9 +20,108 @@ function fireNativeWithDataTransfer(node, type, dataTransfer) {
node.dispatchEvent(event);
}

function triggerEvent(node, event) {
switch (event) {
case 'focusin':
return node.focus();
case 'focusout':
return node.blur();
default:
return node.trigger(event);
}
}

const SUPPORTED_EMBER_EVENTS = {
touchstart: 'touchStart',
touchmove: 'touchMove',
touchend: 'touchEnd',
touchcancel: 'touchCancel',
keydown: 'keyDown',
keyup: 'keyUp',
keypress: 'keyPress',
mousedown: 'mouseDown',
mouseup: 'mouseUp',
contextmenu: 'contextMenu',
click: 'click',
dblclick: 'doubleClick',
focusin: 'focusIn',
focusout: 'focusOut',
submit: 'submit',
input: 'input',
change: 'change',
dragstart: 'dragStart',
drag: 'drag',
dragenter: 'dragEnter',
dragleave: 'dragLeave',
dragover: 'dragOver',
drop: 'drop',
dragend: 'dragEnd',
};

moduleFor(
'EventDispatcher',
class extends RenderingTestCase {
['@test event handler methods are called when event is triggered'](assert) {
let receivedEvent;
let browserEvent;

this.registerComponent('x-button', {
ComponentClass: Component.extend(
{
tagName: 'button',
},
Object.keys(SUPPORTED_EMBER_EVENTS)
.map((browerEvent) => ({
[SUPPORTED_EMBER_EVENTS[browerEvent]](event) {
receivedEvent = event;
},
}))
.reduce((result, singleEventHandler) => ({ ...result, ...singleEventHandler }), {})
),
});

this.render(`{{x-button}}`);

let elementNode = this.$('button');
let element = elementNode[0];

for (browserEvent in SUPPORTED_EMBER_EVENTS) {
receivedEvent = null;
runTask(() => triggerEvent(elementNode, browserEvent));
assert.ok(receivedEvent, `${browserEvent} event was triggered`);
assert.strictEqual(receivedEvent.target, element);
}
}

['@test event listeners are called when event is triggered'](assert) {
let receivedEvent;
let browserEvent;

this.registerComponent('x-button', {
ComponentClass: Component.extend({
tagName: 'button',
init() {
this._super();
Object.keys(SUPPORTED_EMBER_EVENTS).forEach((browserEvent) => {
this.on(SUPPORTED_EMBER_EVENTS[browserEvent], (event) => (receivedEvent = event));
});
},
}),
});

this.render(`{{x-button}}`);

let elementNode = this.$('button');
let element = elementNode[0];

for (browserEvent in SUPPORTED_EMBER_EVENTS) {
receivedEvent = null;
runTask(() => triggerEvent(elementNode, browserEvent));
assert.ok(receivedEvent, `${browserEvent} event was triggered`);
assert.strictEqual(receivedEvent.target, element);
}
}

['@test events bubble view hierarchy for form elements'](assert) {
let receivedEvent;

Expand Down Expand Up @@ -420,12 +519,15 @@ moduleFor(
constructor() {
super(...arguments);

let dispatcher = this.owner.lookup('event_dispatcher:main');
run(dispatcher, 'destroy');
this.owner.__container__.reset('event_dispatcher:main');
Copy link
Contributor Author

@simonihmig simonihmig Oct 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not able to keep this test unchanged, as the reference to the EventDispatcher of the Glimmer Environment (already introduced in the initial PR) would always refer to the first instance, not the second one created here. So the curly component manager would still see the old dispatcher, that has no custom events added.

But I think this affects only our internal testing here, and not imply any publicly visible changes.

this.dispatcher = this.owner.lookup('event_dispatcher:main');
}

getBootOptions() {
return {
skipEventDispatcher: true,
};
}

['@test additional events can be specified'](assert) {
this.dispatcher.setup({ myevent: 'myEvent' });

Expand Down
12 changes: 12 additions & 0 deletions packages/@ember/-internals/views/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Option } from '@glimmer/interfaces';
import { SimpleElement } from '@simple-dom/interface';
import { Object as EmberObject } from '@ember/-internals/runtime';

export { jQuery, jQueryDisabled } from './lib/system/jquery';

Expand Down Expand Up @@ -33,3 +34,14 @@ export const ActionManager: {
[id: string]: any | undefined;
};
};

export declare class EventDispatcher extends EmberObject {
events: Record<string, string>;
finalEventNameMapping: Record<string, string>;
rootElement: string | HTMLElement;
lazyEvents: Map<string, string>;

setup(addedEvents: object, rootElement?: string | HTMLElement): void;
setupHandlerForBrowserEvent(event: string): void;
setupHandlerForEmberEvent(event: string): void;
}
19 changes: 12 additions & 7 deletions packages/@ember/-internals/views/lib/mixins/text_support.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,6 @@ const TextSupport = Mixin.create({
disabled: false,
maxlength: null,

init() {
this._super(...arguments);
this.on('paste', this, this._elementValueDidChange);
this.on('cut', this, this._elementValueDidChange);
this.on('input', this, this._elementValueDidChange);
},

/**
Whether the `keyUp` event that triggers an `action` to be sent continues
propagating to other views.
Expand Down Expand Up @@ -183,6 +176,18 @@ const TextSupport = Mixin.create({
this._elementValueDidChange(event);
},

paste(event) {
this._elementValueDidChange(event);
},

cut(event) {
this._elementValueDidChange(event);
},

input(event) {
this._elementValueDidChange(event);
},

/**
Allows you to specify a controller action to invoke when either the `enter`
key is pressed or, in the case of the field being a textarea, when a newline
Expand Down
Loading