From 87e6531c65394054bfe601d78a7633faff6c52e9 Mon Sep 17 00:00:00 2001 From: Simon Edwards Date: Sun, 1 Aug 2021 20:43:04 +0200 Subject: [PATCH] Re-use existing JS event emitters when creating JS side wrappers By not re-using JS event emitters, if a JS/C++ wrapper is created twice for a `QObject` then any previously registered event handlers will be overwritten and lost when the `QObject`'s `initNodeEventEmitter()` is called for a 2nd time. --- .../nodegui/core/Events/eventwidget_macro.h | 10 ++++ src/lib/core/EventWidget.ts | 51 +++++++++++-------- 2 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/cpp/include/nodegui/core/Events/eventwidget_macro.h b/src/cpp/include/nodegui/core/Events/eventwidget_macro.h index 709bbdb0b..c6985f1a9 100644 --- a/src/cpp/include/nodegui/core/Events/eventwidget_macro.h +++ b/src/cpp/include/nodegui/core/Events/eventwidget_macro.h @@ -22,6 +22,14 @@ return env.Null(); \ } \ \ + Napi::Value getNodeEventEmitter(const Napi::CallbackInfo& info) { \ + Napi::Env env = info.Env(); \ + if (this->instance->emitOnNode) { \ + return this->instance->emitOnNode.Value(); \ + } else { \ + return env.Null(); \ + } \ + } \ Napi::Value subscribeToQtEvent(const Napi::CallbackInfo& info) { \ Napi::Env env = info.Env(); \ Napi::String eventString = info[0].As(); \ @@ -42,6 +50,8 @@ COMPONENT_WRAPPED_METHODS_EXPORT_DEFINE(WidgetWrapName) \ InstanceMethod("initNodeEventEmitter", \ &WidgetWrapName::initNodeEventEmitter), \ + InstanceMethod("getNodeEventEmitter", \ + &WidgetWrapName::getNodeEventEmitter), \ InstanceMethod("subscribeToQtEvent", \ &WidgetWrapName::subscribeToQtEvent), \ InstanceMethod("unSubscribeToQtEvent", \ diff --git a/src/lib/core/EventWidget.ts b/src/lib/core/EventWidget.ts index ced19bc34..3e7c9c6a9 100644 --- a/src/lib/core/EventWidget.ts +++ b/src/lib/core/EventWidget.ts @@ -38,30 +38,37 @@ export abstract class EventWidget extends Component { private _isEventProcessed = false; constructor(native: NativeElement) { super(); - if (native.initNodeEventEmitter) { - this.emitter = new EventEmitter(); - this.emitter.emit = wrapWithActivateUvLoop(this.emitter.emit.bind(this.emitter)); - const logExceptions = (event: string | symbol, ...args: any[]): boolean => { - // Preserve the value of `_isQObjectEventProcessed` as we dispatch this event - // to JS land, and restore it afterwards. This lets us support recursive event - // dispatches on the same object. - const previousEventProcessed = this._isEventProcessed; - this._isEventProcessed = false; - try { - this.emitter.emit(event, ...args); - } catch (e) { - console.log(`An exception was thrown while dispatching an event of type '${event.toString()}':`); - console.log(e); - } - - const returnCode = this._isEventProcessed; - this._isEventProcessed = previousEventProcessed; - return returnCode; - }; - native.initNodeEventEmitter(logExceptions); - } else { + if (native.initNodeEventEmitter == null) { throw new Error('initNodeEventEmitter not implemented on native side'); } + + const preexistingEmitterFunc = native.getNodeEventEmitter(); + if (preexistingEmitterFunc != null) { + this.emitter = preexistingEmitterFunc.emitter; + return; + } + + this.emitter = new EventEmitter(); + this.emitter.emit = wrapWithActivateUvLoop(this.emitter.emit.bind(this.emitter)); + const logExceptions = (event: string | symbol, ...args: any[]): boolean => { + // Preserve the value of `_isQObjectEventProcessed` as we dispatch this event + // to JS land, and restore it afterwards. This lets us support recursive event + // dispatches on the same object. + const previousEventProcessed = this._isEventProcessed; + this._isEventProcessed = false; + try { + this.emitter.emit(event, ...args); + } catch (e) { + console.log(`An exception was thrown while dispatching an event of type '${event.toString()}':`); + console.log(e); + } + + const returnCode = this._isEventProcessed; + this._isEventProcessed = previousEventProcessed; + return returnCode; + }; + logExceptions.emitter = this.emitter; + native.initNodeEventEmitter(logExceptions); addDefaultErrorHandler(native, this.emitter); }