javascript/NativeScript
1
0
Fork 0
mirror of https://github.com/NativeScript/NativeScript.git synced 2025-11-05 13:26:48 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(core): clean up event handling in Observable (#10531)

Browse Source
This commit is contained in:
Jamie Birch
2024-05-02 16:02:01 +09:00
committed by GitHub
parent a91bb7dc18
commit 53e958e623
3 changed files with 218 additions and 151 deletions
Download Patch File Download Diff File Expand all files Collapse all files

60
apps/automated/src/data/observable-tests.ts
View File

@@ -273,7 +273,65 @@ export var test_Observable_removeEventListener_SingleEvent_MultipleCallbacks = f
TKUnit.assert(receivedCount === 3, 'Observable.removeEventListener not working properly with multiple listeners.');
};
export var test_Observable_removeEventListener_MutlipleEvents_SingleCallback = function () {
export var test_Observable_identity = function () {
const obj = new Observable();
let receivedCount = 0;
const callback = () => receivedCount++;
const eventName = Observable.propertyChangeEvent;
// The identity of an event listener is determined by the tuple of
// [eventType, callback, thisArg], and influences addition and removal.
// If you try to add the same callback for a given event name twice, without
// distinguishing by its thisArg, the second addition will no-op.
obj.addEventListener(eventName, callback);
obj.addEventListener(eventName, callback);
obj.set('testName', 1);
TKUnit.assert(receivedCount === 1, 'Expected Observable to fire exactly once upon a property change, having passed the same callback into addEventListener() twice');
obj.removeEventListener(eventName, callback);
TKUnit.assert(!obj.hasListeners(eventName), 'Expected removeEventListener(eventName, callback) to remove all matching callbacks regardless of thisArg');
receivedCount = 0;
// All truthy thisArgs are distinct, so we have three distinct identities here
// and they should all get added.
obj.addEventListener(eventName, callback);
obj.addEventListener(eventName, callback, 1);
obj.addEventListener(eventName, callback, 2);
obj.set('testName', 2);
TKUnit.assert(receivedCount === 3, 'Expected Observable to fire exactly three times upon a property change, having passed the same callback into addEventListener() three times, with the latter two distinguished by each having a different truthy thisArg');
obj.removeEventListener(eventName, callback);
TKUnit.assert(!obj.hasListeners(eventName), 'Expected removeEventListener(eventName, callback) to remove all matching callbacks regardless of thisArg');
receivedCount = 0;
// If you specify thisArg when removing an event listener, it should remove
// just the event listener with the corresponding thisArg.
obj.addEventListener(eventName, callback, 1);
obj.addEventListener(eventName, callback, 2);
obj.set('testName', 3);
TKUnit.assert(receivedCount === 2, 'Expected Observable to fire exactly three times upon a property change, having passed the same callback into addEventListener() three times, with the latter two distinguished by each having a different truthy thisArg');
obj.removeEventListener(eventName, callback, 2);
TKUnit.assert(obj.hasListeners(eventName), 'Expected removeEventListener(eventName, callback, thisArg) to remove just the event listener that matched the callback and thisArg');
obj.removeEventListener(eventName, callback, 1);
TKUnit.assert(!obj.hasListeners(eventName), 'Expected removeEventListener(eventName, callback, thisArg) to remove the remaining event listener that matched the callback and thisArg');
receivedCount = 0;
// All falsy thisArgs are treated alike, so these all have the same identity
// and only the first should get added.
obj.addEventListener(eventName, callback);
obj.addEventListener(eventName, callback, 0);
obj.addEventListener(eventName, callback, false);
obj.addEventListener(eventName, callback, null);
obj.addEventListener(eventName, callback, undefined);
obj.addEventListener(eventName, callback, '');
obj.set('testName', 4);
TKUnit.assert(receivedCount === 1, 'Expected Observable to fire exactly once upon a property change, having passed the same callback into addEventListener() multiple times, each time with a different falsy (and therefore indistinct) thisArg');
obj.removeEventListener(eventName, callback);
TKUnit.assert(!obj.hasListeners(eventName), 'Expected removeEventListener(eventName, callback) to remove all matching callbacks regardless of thisArg');
receivedCount = 0;
};
export var test_Observable_removeEventListener_MultipleEvents_SingleCallback = function () {
var obj = new TestObservable();
var receivedCount = 0;

304
packages/core/data/observable/index.ts
View File

@@ -38,7 +38,7 @@ export interface PropertyChangeData extends EventData {
interface ListenerEntry {
callback: (data: EventData) => void;
thisArg: any;
thisArg?: any;
once?: true;
}
@@ -58,7 +58,7 @@ export class WrappedValue {
/**
* Property which holds the real value.
*/
public wrapped: any
public wrapped: any,
) {}
/**
@@ -81,14 +81,16 @@ export class WrappedValue {
}
}
const _wrappedValues = [new WrappedValue(null), new WrappedValue(null), new WrappedValue(null), new WrappedValue(null), new WrappedValue(null)];
const _wrappedValues = [new WrappedValue(null), new WrappedValue(null), new WrappedValue(null), new WrappedValue(null), new WrappedValue(null)] as const;
const _globalEventHandlers: {
[eventClass: string]: {
[eventName: string]: ListenerEntry[];
[eventName: string]: Array<ListenerEntry>;
};
} = {};
const eventNamesRegex = /\s*,\s*/;
/**
* Observable is used when you want to be notified when a change occurs. Use on/off methods to add/remove listener.
* Please note that should you be using the `new Observable({})` constructor, it is **obsolete** since v3.0,
@@ -106,7 +108,7 @@ export class Observable {
*/
public _isViewBase: boolean;
private readonly _observers: { [eventName: string]: ListenerEntry[] } = {};
private readonly _observers: { [eventName: string]: Array<ListenerEntry> } = {};
/**
* Gets the value of the specified property.
@@ -166,16 +168,7 @@ export class Observable {
* @param thisArg An optional parameter which when set will be used as "this" in callback method call.
*/
public once(event: string, callback: (data: EventData) => void, thisArg?: any): void {
if (typeof event !== 'string') {
throw new TypeError('Event must be string.');
}
if (typeof callback !== 'function') {
throw new TypeError('callback must be function.');
}
const list = this._getEventList(event, true);
list.push({ callback, thisArg, once: true });
this.addEventListener(event, callback, thisArg, true);
}
/**
@@ -191,23 +184,29 @@ export class Observable {
* @param callback A function to be called when some of the specified event(s) is raised.
* @param thisArg An optional parameter which when set will be used as "this" in callback method call.
*/
public addEventListener(eventNames: string, callback: (data: EventData) => void, thisArg?: any): void {
public addEventListener(eventNames: string, callback: (data: EventData) => void, thisArg?: any, once?: boolean): void {
once = once || undefined;
thisArg = thisArg || undefined;
if (typeof eventNames !== 'string') {
throw new TypeError('Events name(s) must be string.');
throw new TypeError('Event name(s) must be a string.');
}
if (typeof callback !== 'function') {
throw new TypeError('callback must be function.');
throw new TypeError('Callback, if provided, must be a function.');
}
const events = eventNames.split(',');
for (let i = 0, l = events.length; i < l; i++) {
const event = events[i].trim();
const list = this._getEventList(event, true);
// TODO: Performance optimization - if we do not have the thisArg specified, do not wrap the callback in additional object (ObserveEntry)
for (const eventName of eventNames.trim().split(eventNamesRegex)) {
const list = this._getEventList(eventName, true);
if (Observable._indexOfListener(list, callback, thisArg) !== -1) {
// Already added.
continue;
}
list.push({
callback: callback,
thisArg: thisArg,
callback,
thisArg,
once,
});
}
}
@@ -219,6 +218,8 @@ export class Observable {
* @param thisArg An optional parameter which when set will be used to refine search of the correct callback which will be removed as event listener.
*/
public removeEventListener(eventNames: string, callback?: (data: EventData) => void, thisArg?: any): void {
thisArg = thisArg || undefined;
if (typeof eventNames !== 'string') {
throw new TypeError('Events name(s) must be string.');
}
@@ -227,137 +228,160 @@ export class Observable {
throw new TypeError('callback must be function.');
}
const events = eventNames.split(',');
for (let i = 0, l = events.length; i < l; i++) {
const event = events[i].trim();
if (callback) {
const list = this._getEventList(event, false);
if (list) {
const index = Observable._indexOfListener(list, callback, thisArg);
if (index >= 0) {
list.splice(index, 1);
}
if (list.length === 0) {
delete this._observers[event];
}
}
} else {
this._observers[event] = undefined;
delete this._observers[event];
for (const eventName of eventNames.trim().split(eventNamesRegex)) {
const entries = this._observers[eventName];
if (!entries) {
continue;
}
Observable.innerRemoveEventListener(entries, callback, thisArg);
if (!entries.length) {
// Clear all entries of this type
delete this._observers[eventName];
}
}
}
public static on(eventName: string, callback: (data: EventData) => void, thisArg?: any): void {
this.addEventListener(eventName, callback, thisArg);
/**
* Please avoid using the static event-handling APIs as they will be removed
* in future.
* @deprecated
*/
public static on(eventName: string, callback: (data: EventData) => void, thisArg?: any, once?: boolean): void {
this.addEventListener(eventName, callback, thisArg, once);
}
/**
* Please avoid using the static event-handling APIs as they will be removed
* in future.
* @deprecated
*/
public static once(eventName: string, callback: (data: EventData) => void, thisArg?: any): void {
if (typeof eventName !== 'string') {
throw new TypeError('Event must be string.');
}
if (typeof callback !== 'function') {
throw new TypeError('callback must be function.');
}
const eventClass = this.name === 'Observable' ? '*' : this.name;
if (!_globalEventHandlers[eventClass]) {
_globalEventHandlers[eventClass] = {};
}
if (!Array.isArray(_globalEventHandlers[eventClass][eventName])) {
_globalEventHandlers[eventClass][eventName] = [];
}
_globalEventHandlers[eventClass][eventName].push({ callback, thisArg, once: true });
this.addEventListener(eventName, callback, thisArg, true);
}
/**
* Please avoid using the static event-handling APIs as they will be removed
* in future.
* @deprecated
*/
public static off(eventName: string, callback?: (data: EventData) => void, thisArg?: any): void {
this.removeEventListener(eventName, callback, thisArg);
}
public static removeEventListener(eventName: string, callback?: (data: EventData) => void, thisArg?: any): void {
if (typeof eventName !== 'string') {
throw new TypeError('Event must be string.');
private static innerRemoveEventListener(entries: Array<ListenerEntry>, callback?: (data: EventData) => void, thisArg?: any): void {
for (let i = 0; i < entries.length; i++) {
const entry = entries[i];
// If we have a `thisArg`, refine on both `callback` and `thisArg`.
if (thisArg && (entry.callback !== callback || entry.thisArg !== thisArg)) {
continue;
}
// If we don't have a `thisArg`, refine only on `callback`.
if (callback && entry.callback !== callback) {
continue;
}
// If we have neither `thisArg` nor `callback`, just remove all events
// of this type regardless.
entries.splice(i, 1);
i--;
}
}
/**
* Please avoid using the static event-handling APIs as they will be removed
* in future.
* @deprecated
*/
public static removeEventListener(eventNames: string, callback?: (data: EventData) => void, thisArg?: any): void {
thisArg = thisArg || undefined;
if (typeof eventNames !== 'string') {
throw new TypeError('Event name(s) must be a string.');
}
if (callback && typeof callback !== 'function') {
throw new TypeError('callback must be function.');
throw new TypeError('Callback, if provided, must be function.');
}
const eventClass = this.name === 'Observable' ? '*' : this.name;
// Short Circuit if no handlers exist..
if (!_globalEventHandlers[eventClass] || !Array.isArray(_globalEventHandlers[eventClass][eventName])) {
return;
}
const events = _globalEventHandlers[eventClass][eventName];
if (thisArg) {
for (let i = 0; i < events.length; i++) {
if (events[i].callback === callback && events[i].thisArg === thisArg) {
events.splice(i, 1);
i--;
}
for (const eventName of eventNames.trim().split(eventNamesRegex)) {
const entries = _globalEventHandlers?.[eventClass]?.[eventName];
if (!entries) {
continue;
}
} else if (callback) {
for (let i = 0; i < events.length; i++) {
if (events[i].callback === callback) {
events.splice(i, 1);
i--;
}
Observable.innerRemoveEventListener(entries, callback, thisArg);
if (!entries.length) {
// Clear all entries of this type
delete _globalEventHandlers[eventClass][eventName];
}
} else {
// Clear all events of this type
delete _globalEventHandlers[eventClass][eventName];
}
if (events.length === 0) {
// Clear all events of this type
delete _globalEventHandlers[eventClass][eventName];
}
// Clear the primary class grouping if no events are left
const keys = Object.keys(_globalEventHandlers[eventClass]);
if (keys.length === 0) {
delete _globalEventHandlers[eventClass];
// Clear the primary class grouping if no list are left
const keys = Object.keys(_globalEventHandlers[eventClass]);
if (keys.length === 0) {
delete _globalEventHandlers[eventClass];
}
}
}
public static addEventListener(eventName: string, callback: (data: EventData) => void, thisArg?: any): void {
if (typeof eventName !== 'string') {
throw new TypeError('Event must be string.');
/**
* Please avoid using the static event-handling APIs as they will be removed
* in future.
* @deprecated
*/
public static addEventListener(eventNames: string, callback: (data: EventData) => void, thisArg?: any, once?: boolean): void {
once = once || undefined;
thisArg = thisArg || undefined;
if (typeof eventNames !== 'string') {
throw new TypeError('Event name(s) must be a string.');
}
if (typeof callback !== 'function') {
throw new TypeError('callback must be function.');
throw new TypeError('Callback must be a function.');
}
const eventClass = this.name === 'Observable' ? '*' : this.name;
if (!_globalEventHandlers[eventClass]) {
_globalEventHandlers[eventClass] = {};
}
if (!Array.isArray(_globalEventHandlers[eventClass][eventName])) {
_globalEventHandlers[eventClass][eventName] = [];
for (const eventName of eventNames.trim().split(eventNamesRegex)) {
if (!_globalEventHandlers[eventClass][eventName]) {
_globalEventHandlers[eventClass][eventName] = [];
}
if (Observable._indexOfListener(_globalEventHandlers[eventClass][eventName], callback, thisArg) !== -1) {
// Already added.
return;
}
_globalEventHandlers[eventClass][eventName].push({ callback, thisArg, once });
}
_globalEventHandlers[eventClass][eventName].push({ callback, thisArg });
}
private _globalNotify<T extends EventData>(eventClass: string, eventType: string, data: T): void {
// Check for the Global handlers for JUST this class
if (_globalEventHandlers[eventClass]) {
const event = data.eventName + eventType;
const events = _globalEventHandlers[eventClass][event];
if (events) {
Observable._handleEvent(events, data);
const eventName = data.eventName + eventType;
const entries = _globalEventHandlers[eventClass][eventName];
if (entries) {
Observable._handleEvent(entries, data);
}
}
// Check for he Global handlers for ALL classes
// Check for the Global handlers for ALL classes
if (_globalEventHandlers['*']) {
const event = data.eventName + eventType;
const events = _globalEventHandlers['*'][event];
if (events) {
Observable._handleEvent(events, data);
const eventName = data.eventName + eventType;
const entries = _globalEventHandlers['*'][eventName];
if (entries) {
Observable._handleEvent(entries, data);
}
}
}
@@ -387,29 +411,27 @@ export class Observable {
}
private static _handleEvent<T extends EventData>(observers: Array<ListenerEntry>, data: T): void {
if (!observers) {
if (!observers.length) {
return;
}
for (let i = observers.length - 1; i >= 0; i--) {
const entry = observers[i];
if (entry) {
if (entry.once) {
observers.splice(i, 1);
}
if (!entry) {
continue;
}
let returnValue: any;
if (entry.thisArg) {
returnValue = entry.callback.apply(entry.thisArg, [data]);
} else {
returnValue = entry.callback(data);
}
if (entry.once) {
observers.splice(i, 1);
}
// This ensures errors thrown inside asynchronous functions do not get swallowed
if (returnValue && returnValue instanceof Promise) {
returnValue.catch((err) => {
console.error(err);
});
}
const returnValue = entry.thisArg ? entry.callback.apply(entry.thisArg, [data]) : entry.callback(data);
// This ensures errors thrown inside asynchronous functions do not get swallowed
if (returnValue instanceof Promise) {
returnValue.catch((err) => {
console.error(err);
});
}
}
}
@@ -443,17 +465,14 @@ export class Observable {
}
public _emit(eventNames: string): void {
const events = eventNames.split(',');
for (let i = 0, l = events.length; i < l; i++) {
const event = events[i].trim();
this.notify({ eventName: event, object: this });
for (const eventName of eventNames.trim().split(eventNamesRegex)) {
this.notify({ eventName, object: this });
}
}
private _getEventList(eventName: string, createIfNeeded?: boolean): Array<ListenerEntry> {
private _getEventList(eventName: string, createIfNeeded?: boolean): Array<ListenerEntry> | undefined {
if (!eventName) {
throw new TypeError('EventName must be valid string.');
throw new TypeError('eventName must be a valid string.');
}
let list = <Array<ListenerEntry>>this._observers[eventName];
@@ -466,20 +485,9 @@ export class Observable {
}
private static _indexOfListener(list: Array<ListenerEntry>, callback: (data: EventData) => void, thisArg?: any): number {
for (let i = 0; i < list.length; i++) {
const entry = list[i];
if (thisArg) {
if (entry.callback === callback && entry.thisArg === thisArg) {
return i;
}
} else {
if (entry.callback === callback) {
return i;
}
}
}
thisArg = thisArg || undefined;
return -1;
return list.findIndex((entry) => entry.callback === callback && entry.thisArg === thisArg);
}
}

5
packages/core/ui/core/bindable/index.ts
View File

@@ -635,8 +635,9 @@ export class Binding {
try {
if (isEventOrGesture(options.property, <any>optionsInstance) && types.isFunction(value)) {
// calling off method with null as handler will remove all handlers for options.property event
optionsInstance.off(options.property, null, optionsInstance.bindingContext);
// Calling Observable.prototype.off() with just the event name will
// remove all handlers under that event name.
optionsInstance.off(options.property);
optionsInstance.on(options.property, value, optionsInstance.bindingContext);
} else if (optionsInstance instanceof Observable) {
optionsInstance.set(options.property, value);