react nodes added #119
react nodes added #119
Conversation
|
@rcraig12 Can you please review Dan's comments on the previous PR? |
| @@ -0,0 +1,137 @@ | |||
| { | |||
| "name": "react/React Dependency Installer", | |||
There was a problem hiding this comment.
Name shouldn't be title case. Suggestion: "react/React dependency installer"
| }, | ||
| "outputs": { | ||
| "82d5b775-4619-466e-bb33-52c9b630500b": { | ||
| "type": "{ \"stdout\" : string}", |
There was a problem hiding this comment.
I think this should simply be of type sting.
The minimal structure of {"error": sting} is only requested for error ports as we plan to have some consistent error structure within Cranq. For the other ports this is not neccessary.
| "name": "stdout" | ||
| }, | ||
| "9c6331a7-5dde-4bb4-a12d-f4a7b1b02ce1": { | ||
| "type": "{ \"stderr\" : string}", |
There was a problem hiding this comment.
Simple string would be sufficient here.
| "name": "error" | ||
| }, | ||
| "4655960f-34c8-41ff-b9e0-3fe400b97c5d": { | ||
| "type": "{ \"bounced\" : string}", |
There was a problem hiding this comment.
The type of the bounced output port should be identical to the type of the input port or the compound type of the input ports if there are more than one. The purpose of this port is to contain all input parameters so that these input parameters are preserved and used for some other operation or even for the same operation after some delay (retry) or after some corrective operations.
In this case the type should be {"dependencies": string[], "projectPath": string}.
The type can even be set dynamically as a reference to the original port types like
{"dependencies": typeof `dependencies`, "projectPath": typeof `projectPath`}
| "description": "Installation path for npm installer.\n" | ||
| }, | ||
| "6668cb7f-874b-4c24-ae30-89fe2e24bad1": { | ||
| "type": "boolean", |
There was a problem hiding this comment.
I'm unsure about the boolean type.
Is there any use case where we would send a signal to this port with a false value? If this port is just a trigger, we usually call it start or use a verb specific to the action of the node like install in this case and the port's type would be any since we expect to receive a signal here in case the node should start its operation and not receive any signal in any other case.
| "name": "error" | ||
| }, | ||
| "4c3736af-6977-4632-b048-5446eaa87edd": { | ||
| "type": "{ \"bounced\" : string}", |
There was a problem hiding this comment.
Should be a type containing all the relevant inputs of the node.
| "inputs": { | ||
| "93078915-107a-4854-baaf-e2cdd4c06760": { | ||
| "type": "string[]", | ||
| "name": "dependencies", |
There was a problem hiding this comment.
Ths internal code node should receive all the required parameters in one port (parent port can sync them together for it) in order to avoid issues due to asynchronous signals.
| "description": "Sends true@\"start\" right after the installer completes successfully." | ||
| }, | ||
| "531c9742-9153-400a-ab29-160e618d4b04": { | ||
| "type": "{ \"stdout\" : string}", |
| "name": "stdout" | ||
| }, | ||
| "c33ef99d-731e-47fc-82da-8bc9ec3cca30": { | ||
| "type": "{ \"stderr\" : string}", |
| "name": "error" | ||
| }, | ||
| "ae982924-274e-4871-9d75-4725bcce2eb2": { | ||
| "type": "{ \"bounced\" : string}", |
There was a problem hiding this comment.
Should be of the same type as the input(s)
|
I have reorganised my repo so that there is no errors during git work as I feel this is why there has been attributes getting messed up so apologies for the fresh pull but I could not see any other way around it :)