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.

Sign up for GitHub

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

Jump to bottom

react nodes #118

Closed
wants to merge 3 commits into from
Closed

react nodes #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
080da42
react nodes
rcraig12 Nov 11, 2022
148c2d7
ammendments to satisfy pull request
rcraig12 Nov 11, 2022
9f1dd96
Syncer changed to sync parameters
rcraig12 Nov 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 127 additions & 0 deletions react/react dependency installer/8f8bd450-3f2b-45a5-abcc-12b633899c77.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
{
"name": "react/React Dependency Installer",
"description": "Installs dependencies defined by the user for the react installer node. Node uses npm to install the dependencies on the same application path as the react application.",
"interface": {
"inputs": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should point out in the descriptions that dependencies and projectPath are not protected from injection attack. (Or validate in code that they are actually path and package names)

"1fc459fc-545e-4675-9916-b94bcab3563c": {
"type": "string[]",
"name": "dependencies",
"description": "Array of npm dependencies to install.\n\neg.\n\n[\"a\",\"b\"]"
},
"9484b15d-ca86-4d63-b07f-cb4f8f21e88b": {
"type": "string",
"name": "projectPath",
"description": "Installation path for npm installer.\n"
},
"6668cb7f-874b-4c24-ae30-89fe2e24bad1": {
"type": "any",
"name": "run",
"description": "Run npm installer if boolean value received is true.\n"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that the type of run is any, it runs on any received value, not just true, and the description should reflect that.

}
},
"outputs": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to makes sure types reflect those of the internal code node. (see comments below)

"82d5b775-4619-466e-bb33-52c9b630500b": {
"type": "any",
"name": "stdout"
},
"9c6331a7-5dde-4bb4-a12d-f4a7b1b02ce1": {
"type": "any",
"name": "stderr"
},
"9c820c27-01f9-4a84-b2d3-a112e05dbb72": {
"type": "{ \"error\" : string}",
"name": "error"
},
"4655960f-34c8-41ff-b9e0-3fe400b97c5d": {
"type": "any",
"name": "bounced"
}
}
},
"implementations": {
"cranq": {
"nodes": {
"1cd55979-d276-4251-8602-b1e7584641e6": {
"coordinates": [
95,
-55
],
"name": "dependency installer",
"prototype": "e4e1b2e6-26a1-4b3b-b472-4464814fa69a",
"params": {}
},
"0726521e-634a-461a-aa98-f4ea527b2bf8": {
"coordinates": [
-54,
14
],
"name": "sync parameters",
"prototype": "222a7543-3692-4c6d-b027-c422c99b23a1",
"params": {
"ef31c1e9-cfd5-458b-8d3a-951e3db9c232": "[\"projectPath\",\"run\"]"
}
}
},
"connections": {
"0726521e-634a-461a-aa98-f4ea527b2bf8": {
"75cde7e3-b158-4836-8bcd-a89e72314ee8": {
"1cd55979-d276-4251-8602-b1e7584641e6": {
"045a62ad-7340-479a-b544-f10d56f16597": {}
}
}
}
},
"forwards": {
"inputs": {
"1fc459fc-545e-4675-9916-b94bcab3563c": {
"1cd55979-d276-4251-8602-b1e7584641e6": {
"93078915-107a-4854-baaf-e2cdd4c06760": {}
}
},
"6668cb7f-874b-4c24-ae30-89fe2e24bad1": {
"0726521e-634a-461a-aa98-f4ea527b2bf8": {
"run": {}
}
},
"9484b15d-ca86-4d63-b07f-cb4f8f21e88b": {
"0726521e-634a-461a-aa98-f4ea527b2bf8": {
"projectPath": {}
}
}
},
"outputs": {
"82d5b775-4619-466e-bb33-52c9b630500b": {
"1cd55979-d276-4251-8602-b1e7584641e6": {
"86e1623a-d67c-4e74-98bb-449e29005d56": {}
}
},
"9c6331a7-5dde-4bb4-a12d-f4a7b1b02ce1": {
"1cd55979-d276-4251-8602-b1e7584641e6": {
"735430f5-61ba-48c6-a8a6-e952edcb259e": {}
}
},
"9c820c27-01f9-4a84-b2d3-a112e05dbb72": {
"1cd55979-d276-4251-8602-b1e7584641e6": {
"8b7a505c-873e-4936-9195-f1b3e28c5520": {}
}
},
"4655960f-34c8-41ff-b9e0-3fe400b97c5d": {
"1cd55979-d276-4251-8602-b1e7584641e6": {
"4c3736af-6977-4632-b048-5446eaa87edd": {}
}
}
}
}
}
},
"iconId": "abac34e3-c832-4c7b-b045-62f82ebc3ddb",
"keywords": {
"react": null,
"npm": null,
"dependency": null,
"installer": null
},
"attributes": {
"locked": true
}
}
51 changes: 51 additions & 0 deletions react/react dependency installer/installer/e4e1b2e6-26a1-4b3b-b472-4464814fa69a.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
{
"implementations": {
"es6-node": {
"inputFunctions": {
"045a62ad-7340-479a-b544-f10d56f16597": "const { exec } = require( \"child_process\" );\nconst { dependencies } = params;\nif (data.start !== false ){\n const command = \"cd \" + data.projectPath + \" && \" + \"npm install \" + params.dependencies.join(' ');\n outputs.stdout( command, tag );\n if (state.running){\n outputs.bounced(data, tag);\n } else {\n state.running = true;\n exec(command, (error, stdout, stderr) => {\n if (error) {\n outputs.error(`${error}`, tag);\n } else {\n outputs.stdout(stdout || \"\", tag);\n outputs.stderr(stderr || \"\", tag);\n outputs.error(null, tag);\n }\n state.running = false;\n });\n }\n}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here data.start !== false will always be satisfied, (when used from the main dependency installer node) as the parent node's syncer adds a run property, not start. I think this condition is unnecessary anyway, as the value carried by start/run is any.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parameters stdout and stderr in the callback of exec can be Buffers, so I'd serialize them the same way as error is serialized.
Also I would not send null on error when there was no error.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the callback error branch, we're sending a string instead of the documented {"error": string} which is the correct type. Should be something like: {"error": String(error)}, or {"error": "Dependencies failed to install", "details": String(error)}.

}
}
},
"interface": {
"inputs": {
"93078915-107a-4854-baaf-e2cdd4c06760": {
"type": "string[]",
"name": "dependencies",
"description": "Array of npm dependencies to install.\n\neg.\n\n[\"a\",\"b\"]"
},
"045a62ad-7340-479a-b544-f10d56f16597": {
"type": "{\n\"projectPath\": string, \n\"start\": boolean\n}",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The start property in this input seems irrelevant as it's only purpose is to convert the projectPath param into a signal.

"name": "projectPath & start",
"description": ""
}
},
"outputs": {
"86e1623a-d67c-4e74-98bb-449e29005d56": {
"type": "any",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be string as we know that's what we send from the code.

"name": "stdout"
},
"735430f5-61ba-48c6-a8a6-e952edcb259e": {
"type": "any",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be string as we know that's what we send from the code.

"name": "stderr"
},
"8b7a505c-873e-4936-9195-f1b3e28c5520": {
"type": "{ \"error\" : string}",
"name": "error"
},
"4c3736af-6977-4632-b048-5446eaa87edd": {
"type": "any",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be "typeof `projectPath & start`" as it simply forwards that input.

"name": "bounced"
}
}
},
"name": "react/React Dependency Installer/Installer",
"iconId": "abac34e3-c832-4c7b-b045-62f82ebc3ddb",
"keywords": {
"": null
},
"description": "see react/React Dependency Installer",
"attributes": {
"internal": true,
"locked": true
}
}
56 changes: 56 additions & 0 deletions react/react installer/e257d740-d47d-4794-82f1-3704850bdba9.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the issues in the dependency installer repeat here, re. port types, serialization of stdout/stderr, etc.
Beyond that I feel the success output is redundant, as we either send something on stdout or error. If I need to turn that into a boolean, I can use a boolean/Reverse condition node.
One other thing I'd like to ask is to wrap this node into a structure node with an identical interface. As a general rule, we give preference to implementing nodes using existing nodes rather than JS, so this would keep the functionality of this node future-proof re. refactoring.

"implementations": {
"es6-node": {
"inputFunctions": {
"df3107d7-179c-4e60-86b4-323813c9e86e": "const fs = require(\"fs\");\r\nconst { exec } = require( \"child_process\" );\r\nconst { replace } = params;\r\nconst command = \"npx create-react-app \" + data + \" --use-npm\";\r\nif (state.running){\r\n outputs.bounced(data, tag);\r\n} else {\r\n state.running = true;\r\n if ( params.replace === true && fs.existsSync(data + \"/.cranq-react-app\") ) {\r\n outputs.stdout(\"Removing react application at \" + data);\r\n fs.rmSync( data, { recursive: true, force: true }, (error) => { \r\n outputs.error({ \"error\" : \"Error deleting react application.\"});\r\n });\r\n }\r\n outputs.stdout( command, tag );\r\n exec(command, (error, stdout, stderr) => {\r\nif (error && (params.replace === false || 'undefined' )) {\r\n outputs.error({ \"error\" : \"Application folder already exists. Set the replace input to true if you wish to reinstall the application from scratch\" }, tag);\r\n outputs.success(false, tag);\r\n} else {\r\n outputs.stdout(stdout || \"\", tag);\r\n outputs.stderr(stderr || \"\", tag);\r\n fs.writeFileSync(data + \"/.cranq-react-app\", \"\");\r\n outputs.success(true, tag);\r\n}\r\n state.running = false;\r\n });\r\n}"
}
}
},
"interface": {
"inputs": {
"df3107d7-179c-4e60-86b4-323813c9e86e": {
"type": "string",
"name": "projectPath",
"description": "Full file path of react application."
},
"32afb2d1-73c5-437c-a82a-8ec997b9a9c1": {
"type": "boolean",
"name": "replace",
"description": "Should the node delete and reinstall the application?\n\nboolean: defaults to false.\n\nUse as a static parameter only."
}
},
"outputs": {
"94074b45-8d35-46d1-8a73-f09c55487709": {
"type": "boolean",
"name": "success",
"description": "Sends true@\"start\" right after the installer completes successfully."
},
"531c9742-9153-400a-ab29-160e618d4b04": {
"type": "any",
"name": "stdout"
},
"c33ef99d-731e-47fc-82da-8bc9ec3cca30": {
"type": "any",
"name": "stderr"
},
"657b3ec0-1b44-4bd6-8a8e-7cf4e903823e": {
"type": "any",
"name": "error"
},
"ae982924-274e-4871-9d75-4725bcce2eb2": {
"type": "any",
"name": "bounced"
}
}
},
"name": "react/React Installer",
"iconId": "abac34e3-c832-4c7b-b045-62f82ebc3ddb",
"keywords": {
"react": null,
"installer": null
},
"description": "Installs a react application to a location set by `projectPath`.",
"attributes": {
"locked": true
}
}