Updating from 0.1.x to 0.2.x broke all components with inputs

[mdegat01]

When updating from v0.1.x to v0.2.x all of my existing components with inputs stopped working. I realized its because inputs moved. Whereas previously the value passed to an input called a would be set to msg.a now it was being set to msg.component.a. It seems this is because of the new global flag added, setting global to true maintains existing behavior and false is the new behavior.

Reading the release notes I see that the intent behind this change is good. The idea is that inputs are now removed after the component finishes rather then being passed back to the caller on the message object. That being said, by defaulting global to false for all existing inputs this release is a major breaking change.

If not too late could you consider changing global to local and inverting this option? This way when someone adds a new component or new input to an existing component you can default local to true but for all existing inputs on existing components local will be false (thus maintaining backwards compatibility).

If it is too late then perhaps could you ad a warning about this to the release notes and/or readme?

[mdegat01]

Actually maybe I'm not understanding the intent behind this change. I made this simple test to see how components work now when one calls another and found this:

Notice how in the third debug message (which comes from the debug node after the leftmost "Use component" node) there is still a component field containing outer: "test", which is what I passed to the input called outer. Is this supposed to be there? I thought turning off global meant these messages were not returned with the msg?

[ollixx]

Hi Mike, thanks for the feedback. After I released, it occurred to me, that this change might annoy people. It's good, that you bring that issue up and I totally agree, that it is a breaking change. My idea to make the best of the situation is to add a switch to the RUN node and let the user decide, wether the parameters can be switched to "local" or "global". The default would be "local", so the existing flows would not break, as the flag is not set there, hence the parameters stay global. I will implement that and would like to ask you to beta test it. I get back to you asap with the solution.

[ollixx]

As for you example: Could you please paste the nodes here, so I can try to reproduce? I am not sure, if there is a wrong part in the messages. Please take a look at my example, that comes with the nodes. Open "import" in the main menu and choose "Examples". There you find the "embedded" example that shows you how nesting components is meant to be.

[mdegat01]

Sure here's the export of my test:

[{"id":"f88b5e2b1095bacd","type":"component_in","z":"f9be3564.a66948","name":"Outer comp","api":[{"name":"outer","type":"string","required":false,"global":false}],"node_is_not_connected":false,"x":110,"y":1240,"wires":[["dfe7726658b6f46f"]]},{"id":"1c57495de727e1da","type":"component_in","z":"f9be3564.a66948","name":"Inner comp","api":[{"name":"inner","type":"string","required":false,"global":false}],"node_is_not_connected":false,"x":110,"y":1340,"wires":[["39b615bd5e972c16","0cf5f04f2304401e"]]},{"id":"e22bc90e82314434","type":"component_out","z":"f9be3564.a66948","name":"Return","mode":"default","node_is_not_connected":false,"component_definitions_are_NOT_allowed_inside_subflows":false,"x":470,"y":1240,"wires":[]},{"id":"39b615bd5e972c16","type":"component_out","z":"f9be3564.a66948","name":"Return","mode":"default","node_is_not_connected":false,"component_definitions_are_NOT_allowed_inside_subflows":false,"x":270,"y":1340,"wires":[]},{"id":"0cf5f04f2304401e","type":"debug","z":"f9be3564.a66948","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"true","targetType":"full","statusVal":"","statusType":"auto","x":270,"y":1300,"wires":[]},{"id":"6e3892fd4bb1b00c","type":"debug","z":"f9be3564.a66948","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"true","targetType":"full","statusVal":"","statusType":"auto","x":470,"y":1200,"wires":[]},{"id":"dfe7726658b6f46f","type":"component","z":"f9be3564.a66948","name":"","targetComponent":{"id":"1c57495de727e1da","name":"Inner comp","api":[{"name":"inner","type":"string","required":false,"global":false}]},"paramSources":{"inner":{"name":"inner","type":"string","required":false,"global":false,"source":"inner test","sourceType":"str"}},"statuz":"","statuzType":"str","outputs":1,"outLabels":["default"],"x":300,"y":1240,"wires":[["e22bc90e82314434","6e3892fd4bb1b00c"]]},{"id":"c408d7c2a3a887f1","type":"component","z":"f9be3564.a66948","name":"","targetComponent":{"id":"f88b5e2b1095bacd","name":"Outer comp","api":[{"name":"outer","type":"string","required":false,"global":false}]},"paramSources":{"outer":{"name":"outer","type":"string","required":false,"global":false,"source":"test outer","sourceType":"str"}},"statuz":"","statuzType":"str","outputs":1,"outLabels":["default"],"x":300,"y":1120,"wires":[["f151336f4cf40fef"]]},{"id":"0cd87abf799e7005","type":"inject","z":"f9be3564.a66948","name":"","props":[{"p":"payload"},{"p":"topic","vt":"str"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","payload":"","payloadType":"date","x":120,"y":1120,"wires":[["c408d7c2a3a887f1"]]},{"id":"f151336f4cf40fef","type":"debug","z":"f9be3564.a66948","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"true","targetType":"full","statusVal":"","statusType":"auto","x":470,"y":1120,"wires":[]}]

[ollixx]

correction: I will put the flag into the START node, obviously

[ollixx]

Sure here's the export of my test:

The result is as expected. You don't change the message at all, so it stays the same, running through both components. Checkout the examples (s.a.)

[mdegat01]

Ok yea I guess I just misunderstood. I had assumed when the message was returned to the original flow that the component part would be removed from msg. It's not a big deal I just thought the point of the global flag was to keep the inputs self-contained space so they were flow local essentially and not have them leak to the caller.

[ollixx]

hmm... no you're right. I oversaw that. The component part is NOT supposed to be there. That's a Bug. Thanks again for reporting. Will fix that asap.

[ollixx]

Hey @mdegat01, I fixed the problem and changed the context to be an option. That way all old flows should still work. I also hope to have fixed the validation problem. If you please, do some testing on the branch - before I create more confusion. I would be thankful. You should be able to install the branch directly into your node-red project: npm install "https://github.com/ollixx/node-red-contrib-components.git#bugfix/validation-context-local" --save If you have questions regarding this, please let me know.

[mdegat01]

Shoot sorry, looks like I took too long, was a busy weekend. Thanks for fixing it though, will give it a shot. 👍

[mdegat01]

Actually that's weird, I can't test it right now. I mean I can still install your branch but my manage pallette menu isn't prompting me to update to version 0.3.2. In fact I can't find this package in node-red's library or on npm anymore. https://flows.nodered.org/node/node-red-contrib-components and https://www.npmjs.com/package/node-red-contrib-components return a 404 right now. Is that expected for a new release or did something go wrong?

[ollixx]

Hey, glad you're back. Today I messed up the npm package. Basically I completely deleted it on npmjs.com. Big Ooops. I am really down. Anyway I can restore the latest version tomorrow. NPM makes me to wait 24h before I can publish there again. It still would be cool, if you could beta test. You cannot install the branch I posted above using the palette in the admin UI. If you want to help, you need to find the node-red directory, from which you actually start the app. It has the node_modules directory in it. When you use a terminal, cd into that directory (you will see the flows.json, setting.json, package.json etc.), you should enter the npm install ... line from above. That will replace the current version with the one from github, i.e. the branch. Then you can restart and run some tests. Let me know, if something is messed up. I already found some more minor bugs. The jump to START node feature is broken and there is a problem in the RETURN node in the editor. I am interested in all weird things you find and will be glad, if you can describe them. Thanks again for helping me.

Cheers :oliver

[mdegat01]

Ok I got it installed. Took some finagling, I actually run Node-RED as a Home Assistant add-on. Not sure if you're familiar with Home Assistant but essentially that means Node-RED is deployed as a docker container and I get limited control over it beyond the configuration options provided to me. Fortunately the docker CLI solves all problems.

This definitely looks much better. I think it makes a lot of sense to put the flag at the top rather then per input as it seems way more likely that people will either make them all local or all global. And it upgraded without issue because existing nodes don't have a value for "Use Local Context" so they continue working as normal off the globals.

I guess my one suggestion would be to default "Use Local Context" to true for new nodes only. This setup works great for existing nodes since its backwards compatible but it would be nice if when I dragged a "start component" node in from the palette it had "Use Local Context" set to true to encourage isolation of inputs as a best practice. But only if its possible to set a default for new nodes that has no effect on existing nodes. If that's not possible then please keep it as is.

Hopefully that last bit made sense. Thanks for your efforts, looks great! Sorry about the npmjs issue, hopefully that gets resolved soon.

[ollixx]

Thanks a lot for helping, I really appreciate it. I fixed some more minor issues, but now the nodes seem pretty OK. I like your idea about "local" being the default for new nodes and keep it "global" for legacy flows. I will try to implement that today, so we can release again. I got no response from NPM yet, but it seems I can still publish newer versions. Somehow the information about the old version still exists. Thanks again!

[ollixx]

Release 0.3.3 is out. Thanks again for your help.