Add tests for registerNodeType

Why:

- Ensure that the function works as expected

This change addresses the need by:

- Add tests
- Remove duplicate logic

.eslintrc.js

@@ -2,7 +2,8 @@ module.exports = {
     "env": {
         "browser": true,
         "es2021": true,
-        "node": true
+        "node": true,
+        "jest/globals": true
     },
     "extends": "eslint:recommended",
     "overrides": [
@@ -11,6 +12,7 @@ module.exports = {
         "ecmaVersion": "latest",
         "sourceType": "module"
     },
+    "plugins": ["jest"],
     "globals": {
         "gl": true,
         "GL": true,

.gitignore

@@ -3,7 +3,7 @@ node_modules/*
 npm-debug.log
 temp/
 temp/*
-coverage/*
+coverage/
 
 # Editors
 /.vscode/*

package.json

@@ -33,6 +33,9 @@
     },
     "homepage": "https://github.com/jagenjo/litegraph.js#readme",
     "devDependencies": {
+        "@types/jest": "^29.5.0",
+        "eslint": "^8.37.0 ",
+        "eslint-plugin-jest": "^27.2.1",
         "express": "^4.17.1",
         "google-closure-compiler": "^20171112.0.0",
         "grunt": "^1.1.0",

src/litegraph.js

@@ -167,60 +167,48 @@
             }
 
             //extend class
-            if (base_class.prototype) {
-                //is a class
-                for (var i in LGraphNode.prototype) {
-                    if (!base_class.prototype[i]) {
-                        base_class.prototype[i] = LGraphNode.prototype[i];
-                    }
+            for (var i in LGraphNode.prototype) {
+                if (!base_class.prototype[i]) {
+                    base_class.prototype[i] = LGraphNode.prototype[i];
                 }
             }
 
             const prev = this.registered_node_types[type];
             if(prev) {
                 console.log("replacing node type: " + type);
-            } else {
-                if( !Object.prototype.hasOwnProperty.call( base_class.prototype, "shape") ) {
-                    Object.defineProperty(base_class.prototype, "shape", {
-                        set: function(v) {
-                            switch (v) {
-                                case "default":
-                                    delete this._shape;
-                                    break;
-                                case "box":
-                                    this._shape = LiteGraph.BOX_SHAPE;
-                                    break;
-                                case "round":
-                                    this._shape = LiteGraph.ROUND_SHAPE;
-                                    break;
-                                case "circle":
-                                    this._shape = LiteGraph.CIRCLE_SHAPE;
-                                    break;
-                                case "card":
-                                    this._shape = LiteGraph.CARD_SHAPE;
-                                    break;
-                                default:
-                                    this._shape = v;
-                            }
-                        },
-                        get: function() {
-                            return this._shape;
-                        },
-                        enumerable: true,
-                        configurable: true
-                    });
-                }
+            }
+            if( !Object.prototype.hasOwnProperty.call( base_class.prototype, "shape") ) {
+                Object.defineProperty(base_class.prototype, "shape", {
+                    set: function(v) {
+                        switch (v) {
+                            case "default":
+                                delete this._shape;
+                                break;
+                            case "box":
+                                this._shape = LiteGraph.BOX_SHAPE;
+                                break;
+                            case "round":
+                                this._shape = LiteGraph.ROUND_SHAPE;
+                                break;
+                            case "circle":
+                                this._shape = LiteGraph.CIRCLE_SHAPE;
+                                break;
+                            case "card":
+                                this._shape = LiteGraph.CARD_SHAPE;
+                                break;
+                            default:
+                                this._shape = v;
+                        }
+                    },
+                    get: function() {
+                        return this._shape;
+                    },
+                    enumerable: true,
+                    configurable: true
+                });
+                
 
-                //warnings
-                if (base_class.prototype.onPropertyChange) {
-                    console.warn(
-                        "LiteGraph node class " +
-                            type +
-                            " has onPropertyChange method, it must be called onPropertyChanged with d at the end"
-                    );
-                }
-
-                //used to know which nodes create when dragging files to the canvas
+                //used to know which nodes to create when dragging files to the canvas
                 if (base_class.supported_extensions) {
                     for (let i in base_class.supported_extensions) {
                         const ext = base_class.supported_extensions[i];
@@ -250,18 +238,8 @@
                         " has onPropertyChange method, it must be called onPropertyChanged with d at the end"
                 );
             }
-
-            //used to know which nodes create when dragging files to the canvas
-            if (base_class.supported_extensions) {
-                for (let i=0; i < base_class.supported_extensions.length; i++) {
-                    var ext = base_class.supported_extensions[i];
-                    if(ext && ext.constructor === String) {
-                        this.node_types_by_file_extension[ ext.toLowerCase() ] = base_class;
-                    }
-                }
-            }
             
-            // TODO one would want to know input and ouput :: this would allow trought registerNodeAndSlotType to get all the slots types
+            // TODO one would want to know input and ouput :: this would allow through registerNodeAndSlotType to get all the slots types
             if (this.auto_load_slot_types) {
                 new base_class(base_class.title || "tmpnode");
             }

src/litegraph.test.js

@@ -1,15 +1,130 @@
-const lg = require("./litegraph");
+describe("register node types", () => {
+    let lg;
+    let calc_sum;
 
-test("Register a node type", () => {
-    function calc_sum(a, b) {
-        return a + b;
-    }
-    lg.LiteGraph.registerNodeType("math/sum", calc_sum);
+    beforeEach(() => {
+        jest.resetModules();
+        lg = require("./litegraph");
+        calc_sum = function calc_sum(a, b) {
+            return a + b;
+        };
+    });
 
-    let node = lg.LiteGraph.registered_node_types["math/sum"];
-    expect(node).toBeTruthy();
-    expect(node.type).toBe("math/sum");
-    expect(node.title).toBe("calc_sum")
-    expect(node.category).toBe("math");
-    expect(node.prototype.configure).toBe(lg.LGraphNode.prototype.configure);
+    afterEach(() => {
+        jest.restoreAllMocks();
+    });
+
+    test("normal case", () => {
+        lg.LiteGraph.registerNodeType("math/sum", calc_sum);
+
+        let node = lg.LiteGraph.registered_node_types["math/sum"];
+        expect(node).toBeTruthy();
+        expect(node.type).toBe("math/sum");
+        expect(node.title).toBe("calc_sum");
+        expect(node.category).toBe("math");
+        expect(node.prototype.configure).toBe(
+            lg.LGraphNode.prototype.configure
+        );
+    });
+
+    test("callback triggers", () => {
+        const consoleSpy = jest
+            .spyOn(console, "log")
+            .mockImplementation(() => {});
+
+        lg.LiteGraph.onNodeTypeRegistered = jest.fn();
+        lg.LiteGraph.onNodeTypeReplaced = jest.fn();
+        lg.LiteGraph.registerNodeType("math/sum", calc_sum);
+        expect(lg.LiteGraph.onNodeTypeRegistered).toHaveBeenCalled();
+        expect(lg.LiteGraph.onNodeTypeReplaced).not.toHaveBeenCalled();
+        lg.LiteGraph.registerNodeType("math/sum", calc_sum);
+        expect(lg.LiteGraph.onNodeTypeReplaced).toHaveBeenCalled();
+        expect(consoleSpy).toHaveBeenCalledWith(
+            expect.stringMatching("replacing node type")
+        );
+        expect(consoleSpy).toHaveBeenCalledWith(
+            expect.stringMatching("math/sum")
+        );
+    });
+
+    test("node with title", () => {
+        calc_sum.title = "The sum title";
+        lg.LiteGraph.registerNodeType("math/sum", calc_sum);
+        let node = lg.LiteGraph.registered_node_types["math/sum"];
+        expect(node.title).toBe("The sum title");
+        expect(node.title).not.toBe(node.name);
+    });
+
+    test("handle error simple object", () => {
+        expect(() =>
+            lg.LiteGraph.registerNodeType("math/sum", { simple: "type" })
+        ).toThrow("Cannot register a simple object");
+    });
+
+    test("check shape mapping", () => {
+        lg.LiteGraph.registerNodeType("math/sum", calc_sum);
+
+        const node_type = lg.LiteGraph.registered_node_types["math/sum"];
+        expect(new node_type().shape).toBe(undefined);
+        node_type.prototype.shape = "default";
+        expect(new node_type().shape).toBe(undefined);
+        node_type.prototype.shape = "box";
+        expect(new node_type().shape).toBe(lg.LiteGraph.BOX_SHAPE);
+        node_type.prototype.shape = "round";
+        expect(new node_type().shape).toBe(lg.LiteGraph.ROUND_SHAPE);
+        node_type.prototype.shape = "circle";
+        expect(new node_type().shape).toBe(lg.LiteGraph.CIRCLE_SHAPE);
+        node_type.prototype.shape = "card";
+        expect(new node_type().shape).toBe(lg.LiteGraph.CARD_SHAPE);
+        node_type.prototype.shape = "custom_shape";
+        expect(new node_type().shape).toBe("custom_shape");
+
+        // Check that also works for replaced node types
+        jest.spyOn(console, "log").mockImplementation(() => {});
+        function new_calc_sum(a, b) {
+            return a + b;
+        }
+        lg.LiteGraph.registerNodeType("math/sum", new_calc_sum);
+        const new_node_type = lg.LiteGraph.registered_node_types["math/sum"];
+        new_node_type.prototype.shape = "box";
+        expect(new new_node_type().shape).toBe(lg.LiteGraph.BOX_SHAPE);
+    });
+
+    test("onPropertyChanged warning", () => {
+        const consoleSpy = jest
+            .spyOn(console, "warn")
+            .mockImplementation(() => {});
+
+        calc_sum.prototype.onPropertyChange = true;
+        lg.LiteGraph.registerNodeType("math/sum", calc_sum);
+        expect(consoleSpy).toBeCalledTimes(1);
+        expect(consoleSpy).toBeCalledWith(
+            expect.stringContaining("has onPropertyChange method")
+        );
+        expect(consoleSpy).toBeCalledWith(expect.stringContaining("math/sum"));
+    });
+
+    test("registering supported file extensions", () => {
+        expect(lg.LiteGraph.node_types_by_file_extension).toEqual({});
+
+        // Create two node types with calc_times overriding .pdf
+        calc_sum.supported_extensions = ["PDF", "exe", null];
+        function calc_times(a, b) {
+            return a * b;
+        }
+        calc_times.supported_extensions = ["pdf", "jpg"];
+        lg.LiteGraph.registerNodeType("math/sum", calc_sum);
+        lg.LiteGraph.registerNodeType("math/times", calc_times);
+
+        expect(
+            Object.keys(lg.LiteGraph.node_types_by_file_extension).length
+        ).toBe(3);
+        expect(lg.LiteGraph.node_types_by_file_extension).toHaveProperty("pdf");
+        expect(lg.LiteGraph.node_types_by_file_extension).toHaveProperty("exe");
+        expect(lg.LiteGraph.node_types_by_file_extension).toHaveProperty("jpg");
+
+        expect(lg.LiteGraph.node_types_by_file_extension.exe).toBe(calc_sum);
+        expect(lg.LiteGraph.node_types_by_file_extension.pdf).toBe(calc_times);
+        expect(lg.LiteGraph.node_types_by_file_extension.jpg).toBe(calc_times);
+    });
 });