Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression introduced in Join node - manual mode should ignore msg.parts #3096

Closed
robinsummerhill opened this issue Jul 29, 2021 · 2 comments
Labels

Comments

@robinsummerhill
Copy link

robinsummerhill commented Jul 29, 2021

Current Behavior

A common pattern we use is to Split an array, use a function node to filter out messages based on certain properties, then join the messages back into an array using a Join node in manual mode.

In previous versions of Node-RED this worked as expected. However, as part of this commit: 4cd9b7b by @dceejay a regression was introduced. (packages/node_modules/@node-red/nodes/core/sequence/17-split.js lines 632-634)

In manual/array mode, the Join node will now use msg.parts.index, if present, to set the index of the current item in the joined array.

This leads to undefined entries in the resulting array where messages have been filtered out.

Expected Behavior

In manual mode the Join node should ignore msg.parts entirely.

Steps To Reproduce

Import the flow below - this splits an array of 10 numbers, filters out the odd values and then joins the remaining values back into an array.

It should output [2,4,6,8,10]

It actually outputs [undefined, 2, undefined, 4, undefined, 6, undefined, 8, undefined,10]

Example flow

[{"id":"0088c846e4d88277","type":"inject","z":"cd6a0c9b.330cf","name":"","props":[{"p":"payload"},{"p":"topic","vt":"str"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","payload":"[1,2,3,4,5,6,7,8,9,10]","payloadType":"json","x":220,"y":820,"wires":[["e5e424cafb6aa788"]]},{"id":"e5e424cafb6aa788","type":"split","z":"cd6a0c9b.330cf","name":"","splt":"\\n","spltType":"str","arraySplt":1,"arraySpltType":"len","stream":false,"addname":"","x":440,"y":820,"wires":[["4a23300370cb28ec"]]},{"id":"4a23300370cb28ec","type":"function","z":"cd6a0c9b.330cf","name":"Filter","func":"// Filter out odd values\nif (msg.payload % 2) {\n    return;\n}\n\n// Uncomment following line to force correct behaviour\n// delete msg.parts\n\nreturn msg;","outputs":1,"noerr":0,"initialize":"","finalize":"","libs":[],"x":640,"y":820,"wires":[["c0adc635d2f1a5ad"]]},{"id":"c0adc635d2f1a5ad","type":"join","z":"cd6a0c9b.330cf","name":"","mode":"custom","build":"array","property":"payload","propertyType":"msg","key":"topic","joiner":"\\n","joinerType":"str","accumulate":false,"timeout":"1","count":"","reduceRight":false,"reduceExp":"","reduceInit":"","reduceInitType":"","reduceFixup":"","x":810,"y":820,"wires":[["a083761fdcf0f3f1"]]},{"id":"a083761fdcf0f3f1","type":"debug","z":"cd6a0c9b.330cf","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"false","statusVal":"","statusType":"auto","x":1010,"y":820,"wires":[]}]

Environment

  • Node-RED version: 2.0.4
  • Node.js version: 14
@knolleary
Copy link
Member

Having reviewed the previous change, we can see there was a piece unrelated to the original issue that had caused this regression.

We've reverted that bit of the fix and added a unit test for your scenario so it shouldn't regress again.

@robinsummerhill
Copy link
Author

Thanks for the quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants