Merge pull request #4668 from node-red/undo-history-fix

Fix undo history of moves and post-deploy handling

CHANGELOG.md

@@ -1,3 +1,10 @@
+#### 3.1.9: Maintenance Release
+
+ - Prevent subflow being added to itself (#4654) @knolleary
+ - Fix use of spawn on windows with cmd files (#4652) @knolleary
+ - Guard refresh of unknown subflow (#4640) @knolleary
+ - Fix subflow module sending messages to debug sidebar (#4642) @knolleary
+
 #### 3.1.8: Maintenance Release
 
  - Add validation and error handling on subflow instance properties (#4632) @knolleary

package.json

@@ -1,6 +1,6 @@
 {
     "name": "node-red",
-    "version": "3.1.8",
+    "version": "3.1.9",
     "description": "Low-code programming for event-driven applications",
     "homepage": "https://nodered.org",
     "license": "Apache-2.0",
@@ -74,7 +74,7 @@
         "passport-oauth2-client-password": "0.1.2",
         "raw-body": "2.5.2",
         "semver": "7.5.4",
-        "tar": "6.1.13",
+        "tar": "6.2.1",
         "tough-cookie": "4.1.3",
         "uglify-js": "3.17.4",
         "uuid": "9.0.0",

packages/node_modules/@node-red/editor-api/package.json

@@ -1,6 +1,6 @@
 {
     "name": "@node-red/editor-api",
-    "version": "3.1.8",
+    "version": "3.1.9",
     "license": "Apache-2.0",
     "main": "./lib/index.js",
     "repository": {
@@ -16,8 +16,8 @@
         }
     ],
     "dependencies": {
-        "@node-red/util": "3.1.8",
-        "@node-red/editor-client": "3.1.8",
+        "@node-red/util": "3.1.9",
+        "@node-red/editor-client": "3.1.9",
         "bcryptjs": "2.4.3",
         "body-parser": "1.20.2",
         "clone": "2.1.2",

packages/node_modules/@node-red/editor-client/package.json

@@ -1,6 +1,6 @@
 {
     "name": "@node-red/editor-client",
-    "version": "3.1.8",
+    "version": "3.1.9",
     "license": "Apache-2.0",
     "repository": {
         "type": "git",

packages/node_modules/@node-red/editor-client/src/js/history.js

@@ -706,11 +706,36 @@ RED.history = (function() {
 
     }
 
+    function markEventDirty (evt) {
+        // This isn't 100% thorough - just covers the main move/edit/delete cases
+        evt.dirty = true
+        if (evt.multi) {
+            for (let i = 0; i < evt.events.length-1; i++) {
+                markEventDirty(evt.events[i])
+            }
+        } else if (evt.t === 'move') {
+            for (let i=0;i<evt.nodes.length;i++) {
+                evt.nodes[i].moved = true
+            }
+        } else if (evt.t === 'edit') {
+            evt.changed = true
+        } else if (evt.t === 'delete') {
+            if (evt.nodes) {
+                for (let i=0;i<evt.nodes.length;i++) {
+                    evt.nodes[i].changed = true
+                }
+            }
+        }
+    }
     return {
-        //TODO: this function is a placeholder until there is a 'save' event that can be listened to
         markAllDirty: function() {
-            for (var i=0;i<undoHistory.length;i++) {
+            // A deploy has happened meaning any undo into the history will represent
+            // an undeployed change - regardless of what it was when the event was recorded.
+            // This goes back through the history any marks them all as being dirty events
+            // and also ensures individual node states are marked dirty
+            for (let i=0;i<undoHistory.length;i++) {
                 undoHistory[i].dirty = true;
+                markEventDirty(undoHistory[i])
             }
         },
         list: function() {

packages/node_modules/@node-red/editor-client/src/js/ui/deploy.js

@@ -612,7 +612,10 @@ RED.deploy = (function() {
                 }
             });
             RED.nodes.eachSubflow(function (subflow) {
-                subflow.changed = false;
+                if (subflow.changed) {
+                    subflow.changed = false;
+                    RED.events.emit("subflows:change", subflow);
+                }
             });
             RED.nodes.eachWorkspace(function (ws) {
                 if (ws.changed || ws.added) {

packages/node_modules/@node-red/editor-client/src/js/ui/editor.js

@@ -1623,8 +1623,8 @@ RED.editor = (function() {
                         }
 
                         if (!isSameObj(old_env, new_env)) {
-                            editing_node.env = new_env;
                             editState.changes.env = editing_node.env;
+                            editing_node.env = new_env;
                             editState.changed = true;
                         }
 

packages/node_modules/@node-red/editor-client/src/js/ui/tab-help.js

@@ -158,8 +158,10 @@ RED.sidebar.help = (function() {
 
     function refreshSubflow(sf) {
         var item = treeList.treeList('get',"node-type:subflow:"+sf.id);
-        item.subflowLabel = sf._def.label().toLowerCase();
-        item.treeList.replaceElement(getNodeLabel({_def:sf._def,type:sf._def.label()}));
+        if (item) {
+            item.subflowLabel = sf._def.label().toLowerCase();
+            item.treeList.replaceElement(getNodeLabel({_def:sf._def,type:sf._def.label()}));
+        }
     }
 
     function hideTOC() {

packages/node_modules/@node-red/editor-client/src/js/ui/view.js

@@ -646,120 +646,128 @@ RED.view = (function() {
                 }
                 d3.event = event;
                 var selected_tool = $(ui.draggable[0]).attr("data-palette-type");
-                var result = createNode(selected_tool);
-                if (!result) {
-                    return;
-                }
-                var historyEvent = result.historyEvent;
-                var nn = RED.nodes.add(result.node);
-
-                var showLabel = RED.utils.getMessageProperty(RED.settings.get('editor'),"view.view-node-show-label");
-                if (showLabel !== undefined &&  (nn._def.hasOwnProperty("showLabel")?nn._def.showLabel:true) && !nn._def.defaults.hasOwnProperty("l")) {
-                    nn.l = showLabel;
-                }
-
-                var helperOffset = d3.touches(ui.helper.get(0))[0]||d3.mouse(ui.helper.get(0));
-                var helperWidth = ui.helper.width();
-                var helperHeight = ui.helper.height();
-                var mousePos = d3.touches(this)[0]||d3.mouse(this);
-
                 try {
-                    var isLink = (nn.type === "link in" || nn.type === "link out")
-                    var hideLabel = nn.hasOwnProperty('l')?!nn.l : isLink;
-
-                    var label = RED.utils.getNodeLabel(nn, nn.type);
-                    var labelParts = getLabelParts(label, "red-ui-flow-node-label");
-                    if (hideLabel) {
-                        nn.w = node_height;
-                        nn.h = Math.max(node_height,(nn.outputs || 0) * 15);
-                    } else {
-                        nn.w = Math.max(node_width,20*(Math.ceil((labelParts.width+50+(nn._def.inputs>0?7:0))/20)) );
-                        nn.h = Math.max(6+24*labelParts.lines.length,(nn.outputs || 0) * 15, 30);
+                    var result = createNode(selected_tool);
+                    if (!result) {
+                        return;
                     }
-                } catch(err) {
-                }
+                    var historyEvent = result.historyEvent;
+                    var nn = RED.nodes.add(result.node);
 
-                mousePos[1] += this.scrollTop + ((helperHeight/2)-helperOffset[1]);
-                mousePos[0] += this.scrollLeft + ((helperWidth/2)-helperOffset[0]);
-                mousePos[1] /= scaleFactor;
-                mousePos[0] /= scaleFactor;
+                    var showLabel = RED.utils.getMessageProperty(RED.settings.get('editor'),"view.view-node-show-label");
+                    if (showLabel !== undefined &&  (nn._def.hasOwnProperty("showLabel")?nn._def.showLabel:true) && !nn._def.defaults.hasOwnProperty("l")) {
+                        nn.l = showLabel;
+                    }
 
-                nn.x = mousePos[0];
-                nn.y = mousePos[1];
+                    var helperOffset = d3.touches(ui.helper.get(0))[0]||d3.mouse(ui.helper.get(0));
+                    var helperWidth = ui.helper.width();
+                    var helperHeight = ui.helper.height();
+                    var mousePos = d3.touches(this)[0]||d3.mouse(this);
 
-                var minX = nn.w/2 -5;
-                if (nn.x < minX) {
-                    nn.x = minX;
-                }
-                var minY = nn.h/2 -5;
-                if (nn.y < minY) {
-                    nn.y = minY;
-                }
-                var maxX = space_width -nn.w/2 +5;
-                if (nn.x > maxX) {
-                    nn.x = maxX;
-                }
-                var maxY = space_height -nn.h +5;
-                if (nn.y > maxY) {
-                    nn.y = maxY;
-                }
+                    try {
+                        var isLink = (nn.type === "link in" || nn.type === "link out")
+                        var hideLabel = nn.hasOwnProperty('l')?!nn.l : isLink;
 
-                if (snapGrid) {
-                    var gridOffset = RED.view.tools.calculateGridSnapOffsets(nn);
-                    nn.x -= gridOffset.x;
-                    nn.y -= gridOffset.y;
-                }
+                        var label = RED.utils.getNodeLabel(nn, nn.type);
+                        var labelParts = getLabelParts(label, "red-ui-flow-node-label");
+                        if (hideLabel) {
+                            nn.w = node_height;
+                            nn.h = Math.max(node_height,(nn.outputs || 0) * 15);
+                        } else {
+                            nn.w = Math.max(node_width,20*(Math.ceil((labelParts.width+50+(nn._def.inputs>0?7:0))/20)) );
+                            nn.h = Math.max(6+24*labelParts.lines.length,(nn.outputs || 0) * 15, 30);
+                        }
+                    } catch(err) {
+                    }
 
-                var linkToSplice = $(ui.helper).data("splice");
-                if (linkToSplice) {
-                    spliceLink(linkToSplice, nn, historyEvent)
-                }
+                    mousePos[1] += this.scrollTop + ((helperHeight/2)-helperOffset[1]);
+                    mousePos[0] += this.scrollLeft + ((helperWidth/2)-helperOffset[0]);
+                    mousePos[1] /= scaleFactor;
+                    mousePos[0] /= scaleFactor;
+
+                    nn.x = mousePos[0];
+                    nn.y = mousePos[1];
+
+                    var minX = nn.w/2 -5;
+                    if (nn.x < minX) {
+                        nn.x = minX;
+                    }
+                    var minY = nn.h/2 -5;
+                    if (nn.y < minY) {
+                        nn.y = minY;
+                    }
+                    var maxX = space_width -nn.w/2 +5;
+                    if (nn.x > maxX) {
+                        nn.x = maxX;
+                    }
+                    var maxY = space_height -nn.h +5;
+                    if (nn.y > maxY) {
+                        nn.y = maxY;
+                    }
+
+                    if (snapGrid) {
+                        var gridOffset = RED.view.tools.calculateGridSnapOffsets(nn);
+                        nn.x -= gridOffset.x;
+                        nn.y -= gridOffset.y;
+                    }
+
+                    var linkToSplice = $(ui.helper).data("splice");
+                    if (linkToSplice) {
+                        spliceLink(linkToSplice, nn, historyEvent)
+                    }
+
+                    var group = $(ui.helper).data("group");
+                    if (group) {
+                        var oldX = group.x; 
+                        var oldY = group.y; 
+                        RED.group.addToGroup(group, nn);
+                        var moveEvent = null;
+                        if ((group.x !== oldX) ||
+                            (group.y !== oldY)) {
+                            moveEvent = {
+                                t: "move",
+                                nodes: [{n: group,
+                                        ox: oldX, oy: oldY,
+                                        dx: group.x -oldX,
+                                        dy: group.y -oldY}],
+                                dirty: true
+                            };
+                        }
+                        historyEvent = {
+                            t: 'multi',
+                            events: [historyEvent],
 
-                var group = $(ui.helper).data("group");
-                if (group) {
-                    var oldX = group.x; 
-                    var oldY = group.y; 
-                    RED.group.addToGroup(group, nn);
-                    var moveEvent = null;
-                    if ((group.x !== oldX) ||
-                        (group.y !== oldY)) {
-                        moveEvent = {
-                            t: "move",
-                            nodes: [{n: group,
-                                     ox: oldX, oy: oldY,
-                                     dx: group.x -oldX,
-                                     dy: group.y -oldY}],
-                            dirty: true
                         };
+                        if (moveEvent) {
+                            historyEvent.events.push(moveEvent)
+                        }
+                        historyEvent.events.push({
+                            t: "addToGroup",
+                            group: group,
+                            nodes: nn
+                        })
                     }
-                    historyEvent = {
-                        t: 'multi',
-                        events: [historyEvent],
 
-                    };
-                    if (moveEvent) {
-                        historyEvent.events.push(moveEvent)
+                    RED.history.push(historyEvent);
+                    RED.editor.validateNode(nn);
+                    RED.nodes.dirty(true);
+                    // auto select dropped node - so info shows (if visible)
+                    clearSelection();
+                    nn.selected = true;
+                    movingSet.add(nn);
+                    updateActiveNodes();
+                    updateSelection();
+                    redraw();
+
+                    if (nn._def.autoedit) {
+                        RED.editor.edit(nn);
+                    }
+                } catch (error) {
+                    if (error.code != "NODE_RED") {
+                        RED.notify(RED._("notification.error",{message:error.toString()}),"error");
+                    } else {
+                        RED.notify(RED._("notification.error",{message:error.message}),"error");
                     }
-                    historyEvent.events.push({
-                        t: "addToGroup",
-                        group: group,
-                        nodes: nn
-                    })
-                }
-
-                RED.history.push(historyEvent);
-                RED.editor.validateNode(nn);
-                RED.nodes.dirty(true);
-                // auto select dropped node - so info shows (if visible)
-                clearSelection();
-                nn.selected = true;
-                movingSet.add(nn);
-                updateActiveNodes();
-                updateSelection();
-                redraw();
-
-                if (nn._def.autoedit) {
-                    RED.editor.edit(nn);
                 }
             }
         });
@@ -2159,9 +2167,9 @@ RED.view = (function() {
                     if (n.ox !== n.n.x || n.oy !== n.n.y || addedToGroup) {
                         // This node has moved or added to a group
                         if (rehomedNodes.has(n)) {
-                            moveAndChangedGroupEvent.nodes.push({...n})
+                            moveAndChangedGroupEvent.nodes.push({...n, moved: n.n.moved})
                         } else {
-                            moveEvent.nodes.push({...n})
+                            moveEvent.nodes.push({...n, moved: n.n.moved})
                         }
                         n.n.dirty = true;
                         n.n.moved = true;
@@ -6063,14 +6071,19 @@ RED.view = (function() {
      function createNode(type, x, y, z) {
         const wasDirty = RED.nodes.dirty()
         var m = /^subflow:(.+)$/.exec(type);
-        var activeSubflow = z ? RED.nodes.subflow(z) : null;
+        var activeSubflow = (z || RED.workspaces.active()) ? RED.nodes.subflow(z || RED.workspaces.active()) : null;
+
         if (activeSubflow && m) {
             var subflowId = m[1];
+            let err
             if (subflowId === activeSubflow.id) {
-                throw new Error(RED._("notification.error", { message: RED._("notification.errors.cannotAddSubflowToItself") }))
+                err = new Error(RED._("notification.errors.cannotAddSubflowToItself"))
+            } else if (RED.nodes.subflowContains(m[1], activeSubflow.id)) {
+                err = new Error(RED._("notification.errors.cannotAddCircularReference"))
             }
-            if (RED.nodes.subflowContains(m[1], activeSubflow.id)) {
-                throw new Error(RED._("notification.error", { message: RED._("notification.errors.cannotAddCircularReference") }))
+            if (err) {
+                err.code = 'NODE_RED'
+                throw err
             }
         }
 

packages/node_modules/@node-red/nodes/core/common/21-debug.html

@@ -378,7 +378,7 @@
                             return { id: id, label: RED.nodes.workspace(id).label } //flow id + name
                         } else {
                             const instanceNode = RED.nodes.node(id)
-                            const pathLabel = (instanceNode.name || RED.nodes.subflow(instanceNode.type.substring(8)).name)
+                            const pathLabel = (instanceNode.name || RED.nodes.subflow(instanceNode.type.substring(8))?.name || instanceNode.type)
                             return { id: id, label: pathLabel }
                         }
                     })

packages/node_modules/@node-red/nodes/core/function/90-exec.js

@@ -20,6 +20,7 @@ module.exports = function(RED) {
     var exec = require('child_process').exec;
     var fs = require('fs');
     var isUtf8 = require('is-utf8');
+    const isWindows = process.platform === 'win32'
 
     function ExecNode(n) {
         RED.nodes.createNode(this,n);
@@ -85,9 +86,12 @@ module.exports = function(RED) {
                         }
                     });
                     var cmd = arg.shift();
+                    // Since 18.20.2/20.12.2, it is invalid to call spawn on Windows with a .bat/.cmd file
+                    // without using shell: true. 
+                    const opts = isWindows ? { ...node.spawnOpt, shell: true } : node.spawnOpt
                     /* istanbul ignore else  */
                     node.debug(cmd+" ["+arg+"]");
-                    child = spawn(cmd,arg,node.spawnOpt);
+                    child = spawn(cmd,arg,opts);
                     node.status({fill:"blue",shape:"dot",text:"pid:"+child.pid});
                     var unknownCommand = (child.pid === undefined);
                     if (node.timer !== 0) {

packages/node_modules/@node-red/nodes/locales/en-US/sequence/17-split.html

@@ -103,7 +103,7 @@
     <h4>Automatic mode</h4>
     <p>Automatic mode uses the <code>parts</code> property of incoming messages to
        determine how the sequence should be joined. This allows it to automatically
-       reverse the action of a <b>split</b> node.
+       reverse the action of a <b>split</b> node.</p>
 
     <h4>Manual mode</h4>
     <p>When configured to join in manual mode, the node is able to join sequences

packages/node_modules/@node-red/nodes/package.json

@@ -1,6 +1,6 @@
 {
     "name": "@node-red/nodes",
-    "version": "3.1.8",
+    "version": "3.1.9",
     "license": "Apache-2.0",
     "repository": {
         "type": "git",

packages/node_modules/@node-red/registry/lib/externalModules.js

@@ -273,7 +273,7 @@ async function installModule(moduleDetails) {
                 let extraArgs = triggerPayload.args || [];
                 let args = ['install', ...extraArgs, installSpec]
                 log.trace(NPM_COMMAND + JSON.stringify(args));
-                return exec.run(NPM_COMMAND, args, { cwd: installDir },true)
+                return exec.run(NPM_COMMAND, args, { cwd: installDir, shell: true },true)
             } else {
                 log.trace("skipping npm install");
             }

packages/node_modules/@node-red/registry/lib/installer.js

@@ -25,12 +25,15 @@ const registryUtil = require("./util");
 const library = require("./library");
 const {exec,log,events,hooks} = require("@node-red/util");
 const child_process = require('child_process');
-const npmCommand = process.platform === 'win32' ? 'npm.cmd' : 'npm';
-let installerEnabled = false;
 
+const isWindows = process.platform === 'win32'
+const npmCommand =  isWindows ? 'npm.cmd' : 'npm';
+
+let installerEnabled = false;
 let settings;
+
 const moduleRe = /^(@[^/@]+?[/])?[^/@]+?$/;
-const slashRe = process.platform === "win32" ? /\\|[/]/ : /[/]/;
+const slashRe = isWindows ? /\\|[/]/ : /[/]/;
 const pkgurlRe = /^(https?|git(|\+https?|\+ssh|\+file)):\/\//;
 const localtgzRe = /^([a-zA-Z]:|\/).+tgz$/;
 
@@ -225,7 +228,7 @@ async function installModule(module,version,url) {
                 let extraArgs = triggerPayload.args || [];
                 let args = ['install', ...extraArgs, installName]
                 log.trace(npmCommand + JSON.stringify(args));
-                return exec.run(npmCommand,args,{ cwd: installDir}, true)
+                return exec.run(npmCommand,args,{ cwd: installDir, shell: true }, true)
             } else {
                 log.trace("skipping npm install");
             }
@@ -260,7 +263,7 @@ async function installModule(module,version,url) {
                 log.warn("------------------------------------------");
                 e = new Error(log._("server.install.install-failed")+": "+err.toString());
                 if (err.hook === "postInstall") {
-                    return exec.run(npmCommand,["remove",module],{ cwd: installDir}, false).finally(() => {
+                    return exec.run(npmCommand,["remove",module],{ cwd: installDir, shell: true }, false).finally(() => {
                         throw e;
                     })
                 }
@@ -356,7 +359,7 @@ async function getModuleVersionFromNPM(module, version) {
     }
 
     return new Promise((resolve, reject) => {
-        child_process.execFile(npmCommand,['info','--json',installName],function(err,stdout,stderr) {
+        child_process.execFile(npmCommand,['info','--json',installName],{ shell: true },function(err,stdout,stderr) {
             try {
                 if (!stdout) {
                     log.warn(log._("server.install.install-failed-not-found",{name:module}));
@@ -511,7 +514,7 @@ function uninstallModule(module) {
                     let extraArgs = triggerPayload.args || [];
                     let args = ['remove', ...extraArgs, module]
                     log.trace(npmCommand + JSON.stringify(args));
-                    return exec.run(npmCommand,args,{ cwd: installDir}, true)
+                    return exec.run(npmCommand,args,{ cwd: installDir, shell: true }, true)
                 } else {
                     log.trace("skipping npm uninstall");
                 }
@@ -578,7 +581,7 @@ async function checkPrereq() {
         installerEnabled = false;
     } else {
         return new Promise(resolve => {
-            child_process.execFile(npmCommand,['-v'],function(err,stdout) {
+            child_process.execFile(npmCommand,['-v'],{ shell: true },function(err,stdout) {
                 if (err) {
                     log.info(log._("server.palette-editor.npm-not-found"));
                     installerEnabled = false;

packages/node_modules/@node-red/registry/package.json

@@ -1,6 +1,6 @@
 {
     "name": "@node-red/registry",
-    "version": "3.1.8",
+    "version": "3.1.9",
     "license": "Apache-2.0",
     "main": "./lib/index.js",
     "repository": {
@@ -16,11 +16,11 @@
         }
     ],
     "dependencies": {
-        "@node-red/util": "3.1.8",
+        "@node-red/util": "3.1.9",
         "clone": "2.1.2",
         "fs-extra": "11.1.1",
         "semver": "7.5.4",
-        "tar": "6.1.13",
+        "tar": "6.2.1",
         "uglify-js": "3.17.4"
     }
 }

packages/node_modules/@node-red/runtime/package.json

@@ -1,6 +1,6 @@
 {
     "name": "@node-red/runtime",
-    "version": "3.1.8",
+    "version": "3.1.9",
     "license": "Apache-2.0",
     "main": "./lib/index.js",
     "repository": {
@@ -16,8 +16,8 @@
         }
     ],
     "dependencies": {
-        "@node-red/registry": "3.1.8",
-        "@node-red/util": "3.1.8",
+        "@node-red/registry": "3.1.9",
+        "@node-red/util": "3.1.9",
         "async-mutex": "0.4.0",
         "clone": "2.1.2",
         "express": "4.19.2",

packages/node_modules/@node-red/util/package.json

@@ -1,6 +1,6 @@
 {
     "name": "@node-red/util",
-    "version": "3.1.8",
+    "version": "3.1.9",
     "license": "Apache-2.0",
     "repository": {
         "type": "git",

packages/node_modules/node-red/package.json

@@ -1,6 +1,6 @@
 {
     "name": "node-red",
-    "version": "3.1.8",
+    "version": "3.1.9",
     "description": "Low-code programming for event-driven applications",
     "homepage": "https://nodered.org",
     "license": "Apache-2.0",
@@ -31,10 +31,10 @@
         "flow"
     ],
     "dependencies": {
-        "@node-red/editor-api": "3.1.8",
-        "@node-red/runtime": "3.1.8",
-        "@node-red/util": "3.1.8",
-        "@node-red/nodes": "3.1.8",
+        "@node-red/editor-api": "3.1.9",
+        "@node-red/runtime": "3.1.9",
+        "@node-red/util": "3.1.9",
+        "@node-red/nodes": "3.1.9",
         "basic-auth": "2.0.1",
         "bcryptjs": "2.4.3",
         "express": "4.19.2",