Resolved side effects from previous inheritance approach
authorAlexander Ebert <ebert@woltlab.com>
Mon, 2 Aug 2021 17:06:53 +0000 (19:06 +0200)
committerAlexander Ebert <ebert@woltlab.com>
Mon, 2 Aug 2021 17:06:53 +0000 (19:06 +0200)
This is a follow-up for db23f8af33398c4851d6ba36436592f406b35a0d which introduced a flawed change.

The iteration over the prototype chain caused the prototype itself being bound to an object on runtime, conflicting with other objects.

The root cause was that some parts of the inherited functions were still bound to `constructed`, which was attempted to be fixed by poking the prototype chain.

This new fix is a bit weird, unless one understands that the call to `Reflect.construct()` is a bit tricky because any bound call inside the constructor of `legacyClass` will be bound to `constructed`.

This change is more of a sledge hammer approach, but it works all cases that I tested, including those that were initially the cause for the previous fix as well as new issues caused by the fix.

ts/WoltLabSuite/Core/Core.ts
wcfsetup/install/files/js/WoltLabSuite/Core/Core.js

index 9d5c0f5b2654ebbf5530b3efbabb94d6d5134e7f..d1a0cedaa4ec5fd17d767c3a0c9d78b671a04f97 100644 (file)
@@ -270,8 +270,6 @@ export function debounce<F extends DebounceCallback>(
   };
 }
 
-const defaultFunctions = Object.getOwnPropertyNames(Object.getPrototypeOf({}));
-
 export function enableLegacyInheritance<T>(legacyClass: T): void {
   (legacyClass as any).call = function (thisValue, ...args) {
     if (window.ENABLE_DEVELOPER_TOOLS) {
@@ -280,16 +278,17 @@ export function enableLegacyInheritance<T>(legacyClass: T): void {
 
     const constructed = Reflect.construct(legacyClass as any, args, thisValue.constructor);
     Object.entries(constructed).forEach(([key, value]) => {
+      if (typeof value === "function") {
+        value = value.bind(thisValue);
+      }
+
       thisValue[key] = value;
     });
 
-    let object = thisValue;
-    while ((object = Object.getPrototypeOf(object))) {
-      Object.getOwnPropertyNames(object).forEach((name) => {
-        if (typeof object[name] === "function" && !defaultFunctions.includes(name)) {
-          object[name] = object[name].bind(thisValue);
-        }
-      });
+    for (const key in thisValue) {
+      if (typeof thisValue[key] === "function") {
+        constructed[key] = thisValue[key].bind(thisValue);
+      }
     }
   };
 }
index 64932b0c8f78a05f7191c922b2829bbb6df1ff14..262a4db7969c7a3c2f5f641c1e971691066f37bb 100644 (file)
@@ -238,7 +238,6 @@ define(["require", "exports"], function (require, exports) {
         };
     }
     exports.debounce = debounce;
-    const defaultFunctions = Object.getOwnPropertyNames(Object.getPrototypeOf({}));
     function enableLegacyInheritance(legacyClass) {
         legacyClass.call = function (thisValue, ...args) {
             if (window.ENABLE_DEVELOPER_TOOLS) {
@@ -246,15 +245,15 @@ define(["require", "exports"], function (require, exports) {
             }
             const constructed = Reflect.construct(legacyClass, args, thisValue.constructor);
             Object.entries(constructed).forEach(([key, value]) => {
+                if (typeof value === "function") {
+                    value = value.bind(thisValue);
+                }
                 thisValue[key] = value;
             });
-            let object = thisValue;
-            while ((object = Object.getPrototypeOf(object))) {
-                Object.getOwnPropertyNames(object).forEach((name) => {
-                    if (typeof object[name] === "function" && !defaultFunctions.includes(name)) {
-                        object[name] = object[name].bind(thisValue);
-                    }
-                });
+            for (const key in thisValue) {
+                if (typeof thisValue[key] === "function") {
+                    constructed[key] = thisValue[key].bind(thisValue);
+                }
             }
         };
     }