Make the wrapper cache generic. Apply it to QWindow and QScreen

src/cpp/include/nodegui/QtWidgets/QWidget/qwidget_macro.h

@@ -13,6 +13,7 @@
 #include "QtGui/QWindow/qwindow_wrap.h"
 #include "QtWidgets/QAction/qaction_wrap.h"
 #include "QtWidgets/QLayout/qlayout_wrap.h"
+#include "core/WrapperCache/wrappercache.h"
 #include "core/YogaWidget/yogawidget_macro.h"
 /*
 
@@ -544,8 +545,7 @@
     Napi::HandleScope scope(env);                                             \
     QWindow* window = this->instance->windowHandle();                         \
     if (window) {                                                             \
-      return QWindowWrap::constructor.New(                                    \
-          {Napi::External<QWindow>::New(env, window)});                       \
+      return WrapperCache::instance.get<QWindow, QWindowWrap>(info, window);  \
     } else {                                                                  \
       return env.Null();                                                      \
     }                                                                         \

src/cpp/include/nodegui/core/WrapperCache/wrappercache.h

@@ -13,6 +13,13 @@ struct CachedObject {
   napi_env env;
 };
 
+/**
+ * C++ side cache for wrapper objects.
+ *
+ * This can cache wrappers for QObjects and uses the Qt "destroyed" signal to
+ * track lifetime and remove objects from the cache. It has a JS side component
+ * `WrapperCache.ts` which can cache the JS side wrapper object.
+ */
 class DLL_EXPORT WrapperCache : public QObject {
   Q_OBJECT
 
@@ -20,15 +27,81 @@ class DLL_EXPORT WrapperCache : public QObject {
   QMap<const QObject*, CachedObject> cache;
 
  public:
-  WrapperCache();
-  Napi::Object get(const Napi::CallbackInfo& info, QScreen* screen);
-
+  /**
+   * Singleton instance. Use this to access the cache.
+   */
   static WrapperCache instance;
-  static Napi::Object init(Napi::Env env, Napi::Object exports);
-  static Napi::Value injectDestroyCallback(const Napi::CallbackInfo& info);
+
+  /**
+   * Get a wrapper for a given Qt object.
+   *
+   * @param T - (template argument) The Qt class of the object being cached,
+   * e.g. `QScreen`.
+   * @param W - (template argument) The wrapper type which matches the object
+   * `QScreenWrap`.
+   * @param object - Pointer to the QObject for which a wrapper is required.
+   * @return The JS wrapper object.
+   */
+  template <class T, class W>
+  Napi::Object get(const Napi::CallbackInfo& info, T* object) {
+    Napi::Env env = info.Env();
+
+    if (this->cache.contains(object)) {
+      napi_value result = nullptr;
+      napi_get_reference_value(this->cache[object].env, this->cache[object].ref,
+                               &result);
+      return Napi::Object(env, result);
+    }
+
+    Napi::HandleScope scope(env);
+    Napi::Object wrapper =
+        W::constructor.New({Napi::External<T>::New(env, object)});
+
+    napi_ref ref = nullptr;
+    napi_create_reference(env, wrapper, 1, &ref);
+    this->cache[object].env = napi_env(env);
+    this->cache[object].ref = ref;
+
+    QObject::connect(object, &QObject::destroyed, this,
+                     &WrapperCache::handleDestroyed);
+    return wrapper;
+  }
+
+  static Napi::Object init(Napi::Env env, Napi::Object exports) {
+    Napi::HandleScope scope(env);
+    exports.Set("WrapperCache_injectCallback",
+                Napi::Function::New<injectDestroyCallback>(env));
+    return exports;
+  }
+
+  static Napi::Value injectDestroyCallback(const Napi::CallbackInfo& info) {
+    Napi::Env env = info.Env();
+    Napi::HandleScope scope(env);
+
+    destroyedCallback = Napi::Persistent(info[0].As<Napi::Function>());
+    return env.Null();
+  }
 
   static Napi::FunctionReference destroyedCallback;
 
  public Q_SLOTS:
-  void handleDestroyed(const QObject* object);
+  void handleDestroyed(const QObject* object) {
+    if (!this->cache.contains(object)) {
+      return;
+    }
+
+    // Callback to JS with the address/ID of the destroyed object. So that it
+    // can clear it out of the cache.
+    if (destroyedCallback) {
+      Napi::Env env = destroyedCallback.Env();
+      Napi::HandleScope scope(env);
+      destroyedCallback.Call(
+          {Napi::Value::From(env, extrautils::hashPointerTo53bit(object))});
+    }
+
+    uint32_t result = 0;
+    napi_reference_unref(this->cache[object].env, this->cache[object].ref,
+                         &result);
+    this->cache.remove(object);
+  }
 };

src/cpp/lib/QtGui/QApplication/qapplication_wrap.cpp

@@ -176,7 +176,7 @@ Napi::Value StaticQApplicationWrapMethods::primaryScreen(
   Napi::HandleScope scope(env);
   auto screen = QApplication::primaryScreen();
   if (screen) {
-    return WrapperCache::instance.get(info, screen);
+    return WrapperCache::instance.get<QScreen, QScreenWrap>(info, screen);
   } else {
     return env.Null();
   }
@@ -191,7 +191,8 @@ Napi::Value StaticQApplicationWrapMethods::screens(
   Napi::Array jsArray = Napi::Array::New(env, screens.size());
   for (int i = 0; i < screens.size(); i++) {
     QScreen* screen = screens[i];
-    auto instance = WrapperCache::instance.get(info, screen);
+    auto instance =
+        WrapperCache::instance.get<QScreen, QScreenWrap>(info, screen);
     jsArray[i] = instance;
   }
   return jsArray;

src/cpp/lib/QtGui/QWindow/qwindow_wrap.cpp

@@ -50,7 +50,7 @@ Napi::Value QWindowWrap::screen(const Napi::CallbackInfo& info) {
 
   QScreen* screen = this->instance->screen();
   if (screen) {
-    return WrapperCache::instance.get(info, screen);
+    return WrapperCache::instance.get<QScreen, QScreenWrap>(info, screen);
   } else {
     return env.Null();
   }

src/cpp/lib/core/WrapperCache/wrappercache.cpp

@@ -5,67 +5,3 @@
 DLL_EXPORT WrapperCache WrapperCache::instance;
 
 Napi::FunctionReference WrapperCache::destroyedCallback;
-
-WrapperCache::WrapperCache() {}
-
-Napi::Object WrapperCache::init(Napi::Env env, Napi::Object exports) {
-  Napi::HandleScope scope(env);
-
-  exports.Set("WrapperCache_injectCallback",
-              Napi::Function::New<injectDestroyCallback>(env));
-  return exports;
-}
-
-Napi::Value WrapperCache::injectDestroyCallback(
-    const Napi::CallbackInfo& info) {
-  Napi::Env env = info.Env();
-  Napi::HandleScope scope(env);
-
-  destroyedCallback = Napi::Persistent(info[0].As<Napi::Function>());
-  return env.Null();
-}
-
-Napi::Object WrapperCache::get(const Napi::CallbackInfo& info,
-                               QScreen* screen) {
-  Napi::Env env = info.Env();
-
-  if (this->cache.contains(screen)) {
-    napi_value result = nullptr;
-    napi_get_reference_value(this->cache[screen].env, this->cache[screen].ref,
-                             &result);
-    return Napi::Object(env, result);
-  }
-
-  Napi::HandleScope scope(env);
-  Napi::Object wrapper =
-      QScreenWrap::constructor.New({Napi::External<QScreen>::New(env, screen)});
-
-  napi_ref ref = nullptr;
-  napi_create_reference(env, wrapper, 1, &ref);
-  this->cache[screen].env = napi_env(env);
-  this->cache[screen].ref = ref;
-
-  QObject::connect(screen, &QObject::destroyed, this,
-                   &WrapperCache::handleDestroyed);
-
-  return wrapper;
-}
-
-void WrapperCache::handleDestroyed(const QObject* object) {
-  if (!this->cache.contains(object)) {
-    return;
-  }
-
-  // Callback to JS with the address/ID of the destroyed object. So that it
-  // can clear it out of the cache.
-  if (destroyedCallback) {
-    Napi::Env env = destroyedCallback.Env();
-    Napi::HandleScope scope(env);
-    destroyedCallback.Call(
-        {Napi::Value::From(env, extrautils::hashPointerTo53bit(object))});
-  }
-
-  uint32_t result = 0;
-  napi_reference_unref(this->cache[object].env, this->cache[object].ref, &result);
-  this->cache.remove(object);
-}

src/lib/QtWidgets/QWidget.ts

@@ -15,10 +15,10 @@ import { QRect } from '../QtCore/QRect';
 import { QObjectSignals } from '../QtCore/QObject';
 import { QFont } from '../QtGui/QFont';
 import { QAction } from './QAction';
-import { QScreen } from '../QtGui/QScreen';
 import memoizeOne from 'memoize-one';
 import { QGraphicsEffect } from './QGraphicsEffect';
 import { QSizePolicyPolicy, QStyle, QWindow } from '../..';
+import { wrapperCache } from '../core/WrapperCache';
 
 /**
 
@@ -426,7 +426,7 @@ export abstract class NodeWidget<Signals extends QWidgetSignals> extends YogaWid
     windowHandle(): QWindow | null {
         const handle = this.native.windowHandle();
         if (handle != null) {
-            return new QWindow(handle);
+            return wrapperCache.get<QWindow>(QWindow, handle);
         }
         return null;
     }

src/lib/core/WrapperCache.ts

@@ -1,4 +1,5 @@
 import addon from '../utils/addon';
+import { NativeElement } from './Component';
 
 /**
  * JS side cache for wrapper objects.
@@ -27,7 +28,7 @@ export class WrapperCache {
         this._cache.delete(objectId);
     }
 
-    get<T>(wrapperConstructor: { new (native: any): T }, native: any): T {
+    get<T>(wrapperConstructor: { new (native: any): T }, native: NativeElement): T {
         const id = native.__id__();
         if (this._cache.has(id)) {
             return this._cache.get(id) as T;

website/docs/development/signal_and_event_handling.md

@@ -91,7 +91,7 @@ Steps:
 
 Inherit from both QPushButton and NodeWidget. Make sure you have added NODEWIDGET_IMPLEMENTATIONS macro. This adds a crucial method for events support. It will override `event(QEvent *)` method of QPushbutton so that nodejs can listen to the events of this widget. This makes sure we convert all the QEvent's of this widget to an event for the nodejs event emitter.
 
-Also make sure to connect all the signals of the widgets to the event emitter instance from NodeJS. This way we kindof convert the signal to a simple nodejs event.
+Also make sure to connect all the signals of the widgets to the event emitter instance from NodeJS. This way we kind of convert the signal to a simple nodejs event.
 
 ```cpp
 #pragma once