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.

Sign up for GitHub

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

Jump to bottom

Refactor on modifier implementation #1573

Merged
merged 1 commit into from
Mar 8, 2024
Merged

Refactor on modifier implementation #1573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions packages/@glimmer/debug/lib/stack-check.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,18 @@ class NullChecker implements Checker<null> {
}
}

class UndefinedChecker implements Checker<undefined> {
declare type: undefined;

validate(value: unknown): value is undefined {
return value === undefined;
}

expected(): string {
return `undefined`;
}
}

class InstanceofChecker<T> implements Checker<T> {
declare type: T;

Expand Down Expand Up @@ -348,6 +360,7 @@ export const CheckBoolean: Checker<boolean> = new TypeofChecker<boolean>('boolea
export const CheckHandle: Checker<number> = CheckNumber;
export const CheckString: Checker<string> = new TypeofChecker<string>('string');
export const CheckNull: Checker<null> = new NullChecker();
export const CheckUndefined: Checker<undefined> = new UndefinedChecker();
export const CheckUnknown: Checker<unknown> = new OpaqueChecker();
export const CheckSafeString: Checker<SafeString> = new SafeStringChecker();
export const CheckObject: Checker<object> = new ObjectChecker();
Expand Down
265 changes: 149 additions & 116 deletions packages/@glimmer/runtime/lib/modifiers/on.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,128 +3,196 @@ import type {
InternalModifierManager,
Owner,
SimpleElement,
UpdatableTag,
} from '@glimmer/interfaces';
import type { UpdatableTag } from '@glimmer/validator';
import { check, CheckFunction, CheckString } from '@glimmer/debug';
import {
check,
CheckBoolean,
CheckFunction,
CheckOr,
CheckString,
CheckUndefined,
} from '@glimmer/debug';
import { registerDestructor } from '@glimmer/destroyable';
import { setInternalModifierManager } from '@glimmer/manager';
import { valueForRef } from '@glimmer/reference';
import { buildUntouchableThis, expect } from '@glimmer/util';
import { assert, buildUntouchableThis } from '@glimmer/util';
import { createUpdatableTag } from '@glimmer/validator';

import { reifyNamed } from '../vm/arguments';

const untouchableContext = buildUntouchableThis('`on` modifier');

interface Listener {
eventName: string;
callback: EventListener;
userProvidedCallback: EventListener;
once: boolean | undefined;
passive: boolean | undefined;
capture: boolean | undefined;
options: AddEventListenerOptions | undefined;
}

export class OnModifierState {
public tag = createUpdatableTag();
public element: Element;
public args: CapturedArguments;
public declare eventName: string;
public declare callback: EventListener;
private declare userProvidedCallback: EventListener;
public once?: boolean | undefined;
public passive?: boolean | undefined;
public capture?: boolean | undefined;
public options?: AddEventListenerOptions | undefined;
public shouldUpdate = true;
public listener: Listener | null = null;

constructor(element: Element, args: CapturedArguments) {
this.element = element;
this.args = args;
}

updateFromArgs(): void {
let { args } = this;

let { once, passive, capture }: AddEventListenerOptions = reifyNamed(args.named);
if (once !== this.once) {
this.once = once;
this.shouldUpdate = true;
}

if (passive !== this.passive) {
this.passive = passive;
this.shouldUpdate = true;
}

if (capture !== this.capture) {
this.capture = capture;
this.shouldUpdate = true;
}
registerDestructor(this, () => {
let { element, listener } = this;
if (listener) {
let { eventName, callback, options } = listener;
removeEventListener(element, eventName, callback, options);
}
});
}

// we want to handle both `true` and `false` because both have a meaning:
// https://bugs.chromium.org/p/chromium/issues/detail?id=770208
if (once !== undefined || passive !== undefined || capture !== undefined) {
this.options = { once, passive, capture } as AddEventListenerOptions;
} else {
this.options = undefined;
}
// Update this.listener if needed
updateListener(): void {
let { element, args, listener } = this;

let first = expect(
assert(
args.positional[0],
'You must pass a valid DOM event name as the first argument to the `on` modifier'
);

let eventName = check(
valueForRef(first),
valueForRef(args.positional[0]),
CheckString,
() => 'You must pass a valid DOM event name as the first argument to the `on` modifier'
);

if (eventName !== this.eventName) {
this.eventName = eventName;
this.shouldUpdate = true;
}

const userProvidedCallbackReference = expect(
assert(
args.positional[1],
'You must pass a function as the second argument to the `on` modifier'
);

const userProvidedCallback = check(
valueForRef(userProvidedCallbackReference),
CheckFunction,
(actual) => {
return `You must pass a function as the second argument to the \`on\` modifier; you passed ${
actual === null ? 'null' : typeof actual
}. While rendering:\n\n${userProvidedCallbackReference.debugLabel ?? `{unlabeled value}`}`;
}
) as EventListener;

if (userProvidedCallback !== this.userProvidedCallback) {
this.userProvidedCallback = userProvidedCallback;
this.shouldUpdate = true;
}
let userProvidedCallback = check(valueForRef(args.positional[1]), CheckFunction, (actual) => {
return `You must pass a function as the second argument to the \`on\` modifier; you passed ${
actual === null ? 'null' : typeof actual
}. While rendering:\n\n${args.positional[1]?.debugLabel ?? `{unlabeled value}`}`;
}) as EventListener;

if (import.meta.env.DEV && args.positional.length !== 2) {
throw new Error(
`You can only pass two positional arguments (event name and callback) to the \`on\` modifier, but you provided ${args.positional.length}. Consider using the \`fn\` helper to provide additional arguments to the \`on\` callback.`
);
}

let needsCustomCallback = import.meta.env.DEV && passive; /* needs passive enforcement */
let once: boolean | undefined = undefined;
let passive: boolean | undefined = undefined;
let capture: boolean | undefined = undefined;

if (import.meta.env.DEV) {
let { once: _once, passive: _passive, capture: _capture, ...extra } = reifyNamed(args.named);

once = check(_once, CheckOr(CheckBoolean, CheckUndefined), (actual) => {
return `You must pass a boolean or undefined as the \`once\` argument to the \`on\` modifier; you passed ${actual}. While rendering:\n\n${
args.named['once']!.debugLabel ?? `{unlabeled value}`
}`;
});

passive = check(_passive, CheckOr(CheckBoolean, CheckUndefined), (actual) => {
return `You must pass a boolean or undefined as the \`passive\` argument to the \`on\` modifier; you passed ${actual}. While rendering:\n\n${
args.named['passive']!.debugLabel ?? `{unlabeled value}`
}`;
});

capture = check(_capture, CheckOr(CheckBoolean, CheckUndefined), (actual) => {
return `You must pass a boolean or undefined as the \`capture\` argument to the \`on\` modifier; you passed ${actual}. While rendering:\n\n${
args.named['capture']!.debugLabel ?? `{unlabeled value}`
}`;
});

if (Object.keys(extra).length > 0) {
throw new Error(
`You can only \`once\`, \`passive\` or \`capture\` named arguments to the \`on\` modifier, but you provided ${Object.keys(
extra
).join(', ')}.`
);
}
} else {
let { once: _once, passive: _passive, capture: _capture } = args.named;

if (_once) {
once = valueForRef(_once) as boolean | undefined;
}

if (_passive) {
passive = valueForRef(_passive) as boolean | undefined;
}

if (_capture) {
capture = valueForRef(_capture) as boolean | undefined;
}
}

let shouldUpdate = false;

if (listener === null) {
shouldUpdate = true;
} else {
shouldUpdate =
eventName !== listener.eventName ||
userProvidedCallback !== listener.userProvidedCallback ||
once !== listener.once ||
passive !== listener.passive ||
capture !== listener.capture;
}

let options: AddEventListenerOptions | undefined = undefined;

// we want to handle both `true` and `false` because both have a meaning:
// https://bugs.chromium.org/p/chromium/issues/detail?id=770208
if (shouldUpdate) {
if (once !== undefined || passive !== undefined || capture !== undefined) {
options = { once, passive, capture } as AddEventListenerOptions;
}
}

if (shouldUpdate) {
let callback = userProvidedCallback;

if (import.meta.env.DEV) {
callback = userProvidedCallback.bind(untouchableContext);

if (this.shouldUpdate) {
if (needsCustomCallback) {
this.callback = function (this: Element, event) {
if (import.meta.env.DEV && passive) {
if (passive) {
let _callback = callback;

callback = (event) => {
event.preventDefault = () => {
throw new Error(
`You marked this listener as 'passive', meaning that you must not call 'event.preventDefault()': \n\n${
userProvidedCallback.name ?? `{anonymous function}`
}`
);
};
}
return userProvidedCallback.call(untouchableContext, event);
};
} else if (import.meta.env.DEV) {
// prevent the callback from being bound to the element
this.callback = userProvidedCallback.bind(untouchableContext);
} else {
this.callback = userProvidedCallback;

return _callback(event);
};
}
}

this.listener = {
eventName,
callback,
userProvidedCallback,
once,
passive,
capture,
options,
};

if (listener) {
removeEventListener(element, listener.eventName, listener.callback, listener.options);
}

addEventListener(element, eventName, callback, options);
}
}
}
Expand Down Expand Up @@ -239,7 +307,7 @@ function addEventListener(
@method on
@public
*/
class OnModifierManager implements InternalModifierManager<OnModifierState | null, object> {
class OnModifierManager implements InternalModifierManager<OnModifierState, object> {
Copy link
Contributor

Choose a reason for hiding this comment

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

it's so nice now!!

getDebugName(): string {
return 'on';
}
Expand All @@ -253,58 +321,23 @@ class OnModifierManager implements InternalModifierManager<OnModifierState | nul
element: SimpleElement | Element,
_state: object,
args: CapturedArguments
): OnModifierState | null {
): OnModifierState {
return new OnModifierState(element as Element, args);
}

getTag(state: OnModifierState | null): UpdatableTag | null {
if (state === null) {
return null;
}

return state.tag;
getTag({ tag }: OnModifierState): UpdatableTag {
return tag;
}

install(state: OnModifierState | null): void {
if (state === null) {
return;
}

state.updateFromArgs();

let { element, eventName, callback, options } = state;

addEventListener(element, eventName, callback, options);

registerDestructor(state, () => removeEventListener(element, eventName, callback, options));

state.shouldUpdate = false;
install(state: OnModifierState): void {
state.updateListener();
}

update(state: OnModifierState | null): void {
if (state === null) {
return;
}

// stash prior state for el.removeEventListener
let { element, eventName, callback, options } = state;

state.updateFromArgs();

if (!state.shouldUpdate) {
return;
}

// use prior state values for removal
removeEventListener(element, eventName, callback, options);

// read updated values from the state object
addEventListener(state.element, state.eventName, state.callback, state.options);

state.shouldUpdate = false;
update(state: OnModifierState): void {
state.updateListener();
}

getDestroyable(state: OnModifierState | null): OnModifierState | null {
getDestroyable(state: OnModifierState): OnModifierState {
return state;
}
}
Expand Down