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

Adds support for event proxying and event options #856

Merged
merged 11 commits into from
Oct 26, 2020
364 changes: 247 additions & 117 deletions package-lock.json

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
"dependencies": {
"@types/cldrjs": "0.4.20",
"@types/globalize": "0.0.34",
"@webcomponents/webcomponentsjs": "1.1.0",
"@webcomponents/webcomponentsjs": "2.5.0",
"cldr-core": "36.0.0",
"cldrjs": "^0.5.0",
"cross-fetch": "3.0.2",
Expand All @@ -65,7 +65,7 @@
"@dojo/scripts": "~4.0.4",
"@types/benchmark": "~1.0.0",
"@types/diff": "3.5.0",
"@types/jsdom": "2.0.*",
"@types/jsdom": "16.2.4",
"@types/multer": "~1.3.3",
"@types/node": "~10.12.10",
"@types/ramda": "0.25.5",
Expand All @@ -79,7 +79,7 @@
"cpx": "~1.5.0",
"eslint": "7.6.0",
"husky": "~0.14.3",
"jsdom": "^9.5.0",
"jsdom": "16.4.0",
"jstat": "^1.7.1",
"lint-staged": "6.0.0",
"multer": "~1.3.0",
Expand Down
21 changes: 21 additions & 0 deletions src/core/has.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,27 @@ add(

add('dom-inert', () => has('host-browser') && Element.prototype.hasOwnProperty('inert'), true);

add(
'dom-passive-event',
() => {
let supportsPassive = false;
if ('host-browser') {
try {
const opts = Object.defineProperty({}, 'passive', {
get() {
supportsPassive = true;
}
});
const f = () => {};
window.addEventListener('testPassive', f, opts);
window.removeEventListener('testPassive', f, opts);
} catch (e) {}
}
return supportsPassive;
},
true
);

add('build-elide', false);

add('test', false);
Expand Down
10 changes: 8 additions & 2 deletions src/core/interfaces.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ export interface AriaAttributes {
'aria-valuetext'?: string;
}

export interface VNodeProperties<T extends EventTarget = EventTarget> extends AriaAttributes {
export interface VNodePropertiesWithoutIndex<T extends EventTarget = EventTarget> extends AriaAttributes {
enterAnimation?: SupportedClassName;

exitAnimation?: SupportedClassName;
Expand All @@ -391,7 +391,6 @@ export interface VNodeProperties<T extends EventTarget = EventTarget> extends Ar
* An object literal like `{height:'100px'}` which allows styles to be changed dynamically. All values must be strings.
*/
readonly styles?: Partial<CSSStyleDeclaration>;

// Pointer Events
onpointermove?(ev: PointerEvent<T>): boolean | void;
onpointerdown?(ev: PointerEvent<T>): boolean | void;
Expand Down Expand Up @@ -479,7 +478,14 @@ export interface VNodeProperties<T extends EventTarget = EventTarget> extends Ar
* determines if the node should be blurred
*/
readonly blur?: boolean | NodeOperationPredicate;
}

type NonUndefined<A> = A extends undefined ? never : A;

type FunctionKeys<T extends object> = { [K in keyof T]-?: NonUndefined<T[K]> extends Function ? K : never }[keyof T];

export interface VNodeProperties<T extends EventTarget = EventTarget> extends VNodePropertiesWithoutIndex<T> {
oneventoptions?: { passive: FunctionKeys<VNodePropertiesWithoutIndex>[] };
/**
* Everything that is not explicitly listed (properties and attributes that are either uncommon or custom).
*/
Expand Down
79 changes: 58 additions & 21 deletions src/core/vdom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1236,6 +1236,8 @@ function wrapFunctionProperties(id: string, properties: any) {
return props;
}

type EventMapValue = { proxy: EventListener; callback: Function; options: any } | undefined;
Copy link
Member

Choose a reason for hiding this comment

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

Could we type options? passive: boolean or similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to actually use EventListenerOptions but I think the type didn't have passive. I'll type it though.


export function renderer(renderer: () => RenderResult): Renderer {
let _mountOptions: MountOptions & { domNode: HTMLElement } = {
sync: false,
Expand All @@ -1248,7 +1250,7 @@ export function renderer(renderer: () => RenderResult): Renderer {
let _processQueue: (ProcessItem | DetachApplication | AttachApplication)[] = [];
let _deferredProcessQueue: (ProcessItem | DetachApplication | AttachApplication)[] = [];
let _applicationQueue: ApplicationInstruction[] = [];
let _eventMap = new WeakMap<Function, EventListener>();
let _eventMap = new WeakMap<Node, { [index: string]: EventMapValue }>();
let _idToWrapperMap = new Map<string, DNodeWrapper>();
let _wrapperSiblingMap = new WeakMap<DNodeWrapper, DNodeWrapper>();
let _idToChildrenWrappers = new Map<string, DNodeWrapper[]>();
Expand Down Expand Up @@ -1282,12 +1284,10 @@ export function renderer(renderer: () => RenderResult): Renderer {
domNode: Node,
eventName: string,
currentValue: (event: Event) => void,
previousValue?: Function
eventOptions?: { passive: string[] }
) {
if (previousValue) {
const previousEvent = _eventMap.get(previousValue);
previousEvent && domNode.removeEventListener(eventName, previousEvent);
}
const proxyEvents = _eventMap.get(domNode) || {};
let proxyEvent = proxyEvents[eventName];

let callback = currentValue;

Expand All @@ -1298,8 +1298,33 @@ export function renderer(renderer: () => RenderResult): Renderer {
};
}

domNode.addEventListener(eventName, callback);
_eventMap.set(currentValue, callback);
const { passive: currentPassive = [] } = eventOptions || {};

const isPassive = currentPassive.indexOf(`on${eventName}`) !== -1;

const options = { passive: isPassive };

if (proxyEvent && proxyEvent.options.passive !== isPassive) {
domNode.removeEventListener(eventName, proxyEvent.proxy);
proxyEvent = undefined;
}

if (proxyEvent) {
proxyEvents[eventName] = { ...proxyEvent, callback };
_eventMap.set(domNode, proxyEvents);
} else {
const proxy = (...args: any[]) => {
const proxyEvents = _eventMap.get(domNode) || {};
const proxyEvent = proxyEvents[eventName];
proxyEvent && proxyEvent.callback(...args);
};
proxyEvents[eventName] = { callback, proxy, options };
has('dom-passive-event')
? domNode.addEventListener(eventName, proxy, options)
: domNode.addEventListener(eventName, proxy);

_eventMap.set(domNode, proxyEvents);
}
}

function removeOrphanedEvents(
Expand All @@ -1312,9 +1337,13 @@ export function renderer(renderer: () => RenderResult): Renderer {
const isEvent = propName.substr(0, 2) === 'on' || onlyEvents;
const eventName = onlyEvents ? propName : propName.substr(2);
if (isEvent && !properties[propName]) {
const eventCallback = _eventMap.get(previousProperties[propName]);
if (eventCallback) {
domNode.removeEventListener(eventName, eventCallback);
const proxyEvents = _eventMap.get(domNode) || {};
let proxyEvent: { proxy: EventListener; callback: Function; options: any } | undefined =
Copy link
Member

Choose a reason for hiding this comment

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

Could this be EventMapValue | undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah it can actually just be inferred, i just forgot to remove it.

proxyEvents[eventName];
if (proxyEvent) {
domNode.removeEventListener(eventName, proxyEvent.proxy);
delete proxyEvents[eventName];
_eventMap.set(domNode, proxyEvents);
}
}
});
Expand Down Expand Up @@ -1609,18 +1638,26 @@ export function renderer(renderer: () => RenderResult): Renderer {
(domNode as any)['select-value'] = propValue;
}
setValue(domNode, propValue, previousValue);
} else if (propName !== 'key' && propValue !== previousValue) {
} else if (propName !== 'key') {
const type = typeof propValue;
if (type === 'function' && propName.lastIndexOf('on', 0) === 0 && includesEventsAndAttributes) {
updateEvent(domNode, propName.substr(2), propValue, previousValue);
} else if (type === 'string' && propName !== 'innerHTML' && includesEventsAndAttributes) {
updateAttribute(domNode, propName, propValue, nextWrapper.namespace);
} else if (propName === 'scrollLeft' || propName === 'scrollTop') {
if ((domNode as any)[propName] !== propValue) {
if (
type === 'function' &&
propName.lastIndexOf('on', 0) === 0 &&
includesEventsAndAttributes &&
(propValue !== previousValue || properties.oneventoptions)
) {
updateEvent(domNode, propName.substr(2), propValue, properties.oneventoptions);
} else if (propName === 'oneventoptions') {
} else if (propValue !== previousValue) {
if (type === 'string' && propName !== 'innerHTML' && includesEventsAndAttributes) {
updateAttribute(domNode, propName, propValue, nextWrapper.namespace);
} else if (propName === 'scrollLeft' || propName === 'scrollTop') {
if ((domNode as any)[propName] !== propValue) {
(domNode as any)[propName] = propValue;
}
} else {
(domNode as any)[propName] = propValue;
}
} else {
(domNode as any)[propName] = propValue;
}
}
}
Expand Down Expand Up @@ -1677,7 +1714,7 @@ export function renderer(renderer: () => RenderResult): Renderer {
}
previousProperties.events = previousProperties.events || {};
Object.keys(events).forEach((event) => {
updateEvent(next.domNode as HTMLElement, event, events[event], previousProperties.events[event]);
updateEvent(next.domNode as HTMLElement, event, events[event]);
});
} else {
setProperties(next.domNode as HTMLElement, previousProperties.properties, next);
Expand Down
9 changes: 4 additions & 5 deletions tests/core/support/jsdom-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ intern.registerPlugin('jsdom', async () => {
* requestAnimationFrame and create a fake document.activeElement getter */
const initialize = () => {
/* Create a basic document */
const doc = jsdom.jsdom(`
const doc = new jsdom.JSDOM(`
<!DOCTYPE html>
<html>
<head></head>
Expand All @@ -42,10 +42,10 @@ intern.registerPlugin('jsdom', async () => {
`);

/* Assign it to the global namespace */
global.document = doc;
global.document = doc.window.document;

/* Assign a global window as well */
global.window = doc.defaultView;
global.window = doc.window;

/* Needed for Pointer Event Polyfill's incorrect Element detection */
global.Element = function() {};
Expand All @@ -67,7 +67,6 @@ intern.registerPlugin('jsdom', async () => {
}
};
};

global.cancelAnimationFrame = () => {};
global.IntersectionObserver = () => {};

Expand All @@ -78,7 +77,7 @@ intern.registerPlugin('jsdom', async () => {
}

global.fakeActiveElement = () => {};
Object.defineProperty(doc, 'activeElement', {
Object.defineProperty(global.document, 'activeElement', {
get: () => {
return global.fakeActiveElement();
}
Expand Down
2 changes: 1 addition & 1 deletion tests/core/support/loadCustomElements.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
intern.registerPlugin('custom-elements', async function() {
const scripts = ['./node_modules/@webcomponents/webcomponentsjs/webcomponents-lite.js'];
const scripts = ['./node_modules/@webcomponents/webcomponentsjs/webcomponents-bundle.js'];
if (window.customElements) {
scripts.unshift('./node_modules/@webcomponents/webcomponentsjs/custom-elements-es5-adapter.js');
}
Expand Down
78 changes: 77 additions & 1 deletion tests/core/unit/vdom.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const { assert } = intern.getPlugin('chai');
const { describe: jsdomDescribe } = intern.getPlugin('jsdom');
import global from '../../../src/shim/global';
import { from as arrayFrom } from '../../../src/shim/array';
import { spy, stub, SinonSpy, SinonStub } from 'sinon';
import { spy, stub, createSandbox, SinonSpy, SinonStub, SinonSandbox } from 'sinon';
import { add } from '../../../src/core/has';
import { createResolvers } from './../support/util';
import sendEvent from '../support/sendEvent';
Expand Down Expand Up @@ -6186,6 +6186,13 @@ jsdomDescribe('vdom', () => {
});

describe('events', () => {
let sandbox: SinonSandbox;
beforeEach(() => {
sandbox = createSandbox();
});
afterEach(() => {
sandbox.restore();
});
it('should add an event listener', () => {
const onclick = stub();
const renderFunction = () => {
Expand Down Expand Up @@ -6306,6 +6313,75 @@ jsdomDescribe('vdom', () => {
assert.strictEqual(typedKeys, 'ab');
meta.setRenderResult(renderFunction());
});

it('should support passive oneventoptions', () => {
let addEventListenerSpy: SinonStub;
const createElement = document.createElement.bind(document);
const createElementStub = sandbox.stub(document, 'createElement');
createElementStub.callsFake((name: string) => {
const element = createElement(name);
addEventListenerSpy = stub(element, 'addEventListener');
return element;
});

let invalidate: () => void;
let passive = true;
const onscroll = () => {};

const MyWidget = create({ invalidator })(({ middleware: { invalidator } }) => {
invalidate = invalidator;
return (
<div onscroll={onscroll} oneventoptions={{ passive: passive ? ['onscroll'] : [] }}>
Hello
</div>
);
});

const root: HTMLElement = document.createElement('div');
const r = renderer(() => <MyWidget />);

// force support of passive events
add('dom-passive-event', true, true);
r.mount({ domNode: root, sync: true });

let [, , eventOptions] = addEventListenerSpy!.firstCall.args;
assert.deepEqual(eventOptions, { passive: true });
passive = false;
invalidate!();
[, , eventOptions] = addEventListenerSpy!.secondCall.args;
assert.deepEqual(eventOptions, { passive: false });

// force non-support of passive events
add('dom-passive-event', false, true);
passive = true;
invalidate!();
[, , eventOptions] = addEventListenerSpy!.thirdCall.args;
assert.deepEqual(eventOptions, undefined);
});

it('should not re-attach event listeners for the same type if the callback changes', () => {
let addEventListenerSpy: SinonStub;
const createElement = document.createElement.bind(document);
const createElementStub = sandbox.stub(document, 'createElement');
createElementStub.callsFake((name: string) => {
const element = createElement(name);
addEventListenerSpy = stub(element, 'addEventListener');
return element;
});

let invalidate: () => void;
const MyWidget = create({ invalidator })(({ middleware: { invalidator } }) => {
invalidate = invalidator;
return <div onclick={() => {}}>Hello</div>;
});

const root: HTMLElement = document.createElement('div');
const r = renderer(() => <MyWidget />);
r.mount({ domNode: root, sync: true });
assert.equal(addEventListenerSpy!.callCount, 1);
invalidate!();
assert.equal(addEventListenerSpy!.callCount, 1);
});
});

describe('children', () => {
Expand Down