feat: JS Wrapper Around The Polywrap CLI

[dOrgJelli]

This PR creates a JS SDK that wraps the CLI, allowing developers to more easily script against the polywrap CLI. This has been previously discussed here: #1343

The @polywrap/cli-js package spawns the polywrap CLI as a child-process. This is done for the following reasons:

• Makes it easy to run multiple tasks in parallel.
• Forms a clear dividing line between parent & child process, so you can better control things like system-level restrictions.
• The CLI already assumes it is running in a separate process, 1 command = 1 process life-time, so changing this assumption could cause some code to not run well (maintain bad state between runs, etc).
• Allows us to recreate the CLI in another language that isn't JS.

Moreover, this JS SDK aims to be completely type-safe, and tied to the typings used within the CLI command's themselves. We've written it in a way that the types flow easily from the command's source files, into the JS SDK, see here:
https://github.com/polywrap/toolchain/blob/ece1d33f7af4b9f22b7405fc15ac4d3435667a2c/packages/cli/src/commands/index.ts#L29-L66

And here:
https://github.com/polywrap/toolchain/blob/ece1d33f7af4b9f22b7405fc15ac4d3435667a2c/packages/js/cli/src/commands/index.ts#L30-L54

This gives us an easy to use (typed) interface like so:

await Commands.build({
  manifestFile: "...",
  outputDir: "...",
  codegen: true
}, {
  cwd: "/path/to/project",
  env: {
    CUSTOM_ENV_VAR: "..."
  }
});

[pileks]

All in all, looks amazing! I’d want to see my 2nd comment be addressed, but it’s good either way.

[lgtm-com]

This pull request introduces 20 alerts when merging 039b836 into 46446f0 - view on LGTM.com

new alerts:

• 19 for Unsafe shell command constructed from library input
• 1 for Comparison between inconvertible types

[lgtm-com]

This pull request introduces 20 alerts when merging 9ada2ce into 46446f0 - view on LGTM.com

new alerts:

• 19 for Unsafe shell command constructed from library input
• 1 for Comparison between inconvertible types

[cbrzn]

Hey @dOrgJelli would it be possible to fix LGTM alerts before merging this?

[pileks]

Overall a LGTM from me!

[Niraj-Kamdar]

I agree about exposing the functionality of CLI with JS API, but from what I believe most of the tool that provide the API lib in native language exposes the underlying API methods that CLI would run, they doesn't generally wrap the CLI with exec because user can easily do this if they want to by themselves.

[dOrgJelli]

Hey @Niraj-Kamdar, the reasons for having the JS SDK spawning the CLI as a child process are as follows:

• Makes it easy to run multiple tasks in parallel.
• Forms a clear dividing line between parent & child process, so you can better control things like system-level restrictions.
• The CLI already assumes it is running in a separate process, 1 command = 1 process life-time, so changing this assumption could cause some code to not run well (maintain bad state between runs, etc).
• Allows us to recreate the CLI in another language that isn't JS.

Also the point of this package is to allow users to easily script against the CLI in JS, while maintaining type safety with the CLI's commands & their arguments + options. We've written it in a way that the types flow easily from the command's source files, into the JS SDK, see here:
https://github.com/polywrap/toolchain/blob/ece1d33f7af4b9f22b7405fc15ac4d3435667a2c/packages/cli/src/commands/index.ts#L29-L66

And here:
https://github.com/polywrap/toolchain/blob/ece1d33f7af4b9f22b7405fc15ac4d3435667a2c/packages/js/cli/src/commands/index.ts#L30-L54

This gives us an easy to use (typed) interface like so:

await Commands.build({
  manifestFile: "...",
  outputDir: "...",
  codegen: true
}, {
  cwd: "/path/to/project",
  env: {
    CUSTOM_ENV_VAR: "..."
  }
});

Given all of this, would you agree this package is useful and implemented well?

[pileks]

Other than the comments already listed out, looks good!

The explanation you gave to Niraj helps a lot too.

I'd advise that you reuse that comment to edit the PR description so that we have an immediate reference for this PR (without scrolling through tons of comments).

[dOrgJelli]

@pileks description updated! Thanks for the recommendation.

[Niraj-Kamdar]

LGTM!