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

Event listeners cleaner #165

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
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
85 changes: 65 additions & 20 deletions src/elements.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
export abstract class BaseView<T extends HTMLElement|SVGElement> {
readonly _data: Obj<unknown> = {};
readonly _events: Obj<EventCallback[]> = {};
_mutationObserver: MutationObserver|undefined;
private readonly _mutationObserverCallbacks: Obj<EventCallback[]> = {};
readonly type: string = 'default';
model?: Observable;

Expand Down Expand Up @@ -592,12 +594,31 @@
}
}

/** Removes listeners and model data from el */
private unsubscribe() {
this.model?.clear();
this._mutationObserver?.disconnect();
this.offAll();

for (const child of this.children) {
child.unsubscribe();
}

// hack to avoid TS notification
// about setting undefined value to readonly properties
joshestein marked this conversation as resolved.
Show resolved Hide resolved
delete (this as any)._el;

Check warning on line 609 in src/elements.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test on Node 16.x

Unexpected any. Specify a different type

Check warning on line 609 in src/elements.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test on Node 18.x

Unexpected any. Specify a different type
delete (this as any)._data;

Check warning on line 610 in src/elements.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test on Node 16.x

Unexpected any. Specify a different type

Check warning on line 610 in src/elements.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test on Node 18.x

Unexpected any. Specify a different type
delete (this as any)._events;

Check warning on line 611 in src/elements.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test on Node 16.x

Unexpected any. Specify a different type

Check warning on line 611 in src/elements.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test on Node 18.x

Unexpected any. Specify a different type

// remove mutation observer instances
delete (this as any)._mutationObserver;

Check warning on line 614 in src/elements.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test on Node 16.x

Unexpected any. Specify a different type

Check warning on line 614 in src/elements.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test on Node 18.x

Unexpected any. Specify a different type
delete (this as any)._mutationObserverCallbacks;

Check warning on line 615 in src/elements.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test on Node 16.x

Unexpected any. Specify a different type

Check warning on line 615 in src/elements.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test on Node 18.x

Unexpected any. Specify a different type
}

/** Removes this element. */
remove() {
this.detach();
// TODO Remove event listeners (including children)
// TODO Remove model bindings (including children)
// this._el = this._data = this._events = undefined;
this.unsubscribe();
}

/** Removes all children of this element. */
Expand Down Expand Up @@ -643,13 +664,24 @@
*/
off(events: string, callback?: EventCallback) {
for (const e of words(events)) {
if (e in this._events) {
this._events[e] = callback ? this._events[e].filter(fn => fn !== callback) : [];
if (callback) {
this._events[e] = this._events[e].filter(fn => fn !== callback);
unbindEvent(this, e, callback);
continue;
}
unbindEvent(this, e, callback);
for (const eventsCallback of this._events[e]) unbindEvent(this, e, eventsCallback);
}
}

/**
* Removes all event listeners from this element
*/
offAll() {
Object.entries(this._events).forEach(([eventName, callbacks]) => {
callbacks.forEach((callback) => this.off(eventName, callback));
});
}

/** Triggers a specific event on this element. */
trigger(events: string, args: unknown = {}) {
for (const e of words(events)) {
Expand All @@ -667,28 +699,41 @@
const keyNames = new Set(words(keys));
const event = options?.up ? 'keyup' : 'keydown';

const target = (this._el === document.body ? document : this._el) as HTMLElement;
target.addEventListener(event, (e: KeyboardEvent) => {
const eventFunction = (e: KeyboardEvent) => {
const key = keyCode(e);
if (options?.meta ? !e.ctrlKey && !e.metaKey : e.ctrlKey || e.metaKey) return;
if (!key || !keyNames.has(key)) return;
if (document.activeElement !== this._el && document.activeElement?.shadowRoot?.activeElement !== this._el && Browser.formIsActive) return;
callback(e as KeyboardEvent, key);
});
};

const target = (this._el === document.body ? document : this._el) as HTMLElement;
target.addEventListener(event, eventFunction);

if (!(event in this._events)) this._events[event] = [];
this._events[event].push(eventFunction);
}

/**
* Bind an listener when element attribute changed
*/
onAttr(name: string, callback: (value: string, initial?: boolean) => void) {
// TODO Reuse existing observers, remove events, disconnect when deleting.

const observer = new MutationObserver((mutations) => {
for (const m of mutations) {
if (m.type === 'attributes' && m.attributeName === name) {
callback(this.attr(name));
if (!this._mutationObserver) {
this._mutationObserver = new MutationObserver((mutations) => {
for (const m of mutations) {
if (m.type === 'attributes' && m.attributeName === name) {
for (const attributeCallback of this._mutationObserverCallbacks[name]) {
attributeCallback(this.attr(name));
}
}
}
}
});
});
this._mutationObserver.observe(this._el, {attributes: true});
}

if (!(name in this._mutationObserverCallbacks)) this._mutationObserverCallbacks[name] = [];
this._mutationObserverCallbacks[name].push(callback);

observer.observe(this._el, {attributes: true});
callback(this.attr(name), true);
}

Expand Down Expand Up @@ -994,7 +1039,7 @@
if (!obj) return this.setAttr('d', '');
const attributes: SVGDrawingOptions = {};
for (const p of ['mark', 'arrows', 'round'] as const) {
if (this.hasAttr(p)) attributes[p] = this.attr(p) as any;

Check warning on line 1042 in src/elements.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test on Node 16.x

Unexpected any. Specify a different type

Check warning on line 1042 in src/elements.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test on Node 18.x

Unexpected any. Specify a different type
}
if (this.hasClass('fill')) attributes.fill = 'fill';
if (this.hasAttr('size')) attributes.size = (+this.attr('size')) || undefined;
Expand Down Expand Up @@ -1140,7 +1185,7 @@
}

get scrollTop() {
return window.pageYOffset;
return window.scrollY;
}

set scrollTop(y) {
Expand All @@ -1149,7 +1194,7 @@
}

get scrollLeft() {
return window.pageXOffset;
return window.scrollX;
}

set scrollLeft(x) {
Expand Down
1 change: 1 addition & 0 deletions src/observable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ export function observe<T extends object = any>(state: T, parentModel?: Observab
state = {} as T;
callbackMap.clear();
computedKeys.clear();
watchAllCallbacks.clear();
joshestein marked this conversation as resolved.
Show resolved Hide resolved
lastKey = 0;
}

Expand Down
Loading