RFC: Better DX for making CLI

jaredpalmer commented 4 years ago

Current Behavior

TSDX doesn't really support building CLI's well.

The preserve-shebang plugin doesn't actually work (#109) because we insert a CJS entry file that doesn't know about it. Thus, you actually can't create a CLI package without manually inserting #!/usr/bin/env node in front of ./dist/index.js. I recently did this in a lil postbuild script. Not bad, but also not great.
Dev/prod doesn't matter to CLI's, neither does esm or umd.

Desired Behavior

Building CLI's should be seamless and intuitive

Who does this impact? Who is this for?

Me, advanced users

Describe alternatives you've considered

Additional context

agilgur5 commented 4 years ago

The preserve-shebang plugin doesn't actually work (#109)

I'm not sure what the exact issue is in #109 , but that was implemented there just stripped them out in #117. The most up-to-date shebang plugin I've seen is elado/rollup-plugin-preserve-shebangs, which also has tests.

But just replacing the shebang plugin still runs into #338. I had actually been doing some work to fix #338 by converting the dist/index.js file creation into its own Rollup plugin (which would also resolve https://github.com/formium/tsdx/issues/351#issuecomment-657284291, but ran into blockers as it depends on two different outputs. You can rely on convention but that has some issues as it wouldn't use inputs. Haven't had time to work that out but #338 is a top issue. As an aside, the current plugin also isn't inserting anything right now, so there's two steps there.

Dev/prod doesn't matter to CLI's, neither does esm or umd.

A few things to unpack here. Dev/prod opt-out is an existing issue: #508 . The solution is not difficult, but it means adding yet another flag. The use-cases that one has for browser/React dev/prod can also apply to CLIs if you flip NODE_ENV. Debug modes and such can be useful. UMD isn't, but ESM or mjs more specifically (which is not quite the same thing) is relevant in newer versions of Node.

But yea it's not a primary use-case or quite as useful in a CLI, or generically in server-side code.

Explore https://github.com/vercel/ncc and how it inlines all dependencies into a single file. Evaluate whether we take its best ideas or just document how to integrate it as a recipe

ncc's use-case is primarily serverless, though I've also seen it used in GH Actions (like in upstream @bahmutov/npm-install which I've been contributing to). CLIs don't have to be packaged into one file if they're installed with Node and ideally shouldn't be so you're not duplicating dependencies. TSDX itself isn't one file. Also to put everything into one file is just a configuration in Rollup, setting external to always return false / removing external as you've previously mentioned in https://github.com/formium/tsdx/issues/420#issuecomment-573715129 and I have in https://github.com/formium/tsdx/issues/179#issuecomment-605502952 .

Did you have something in mind other than that? I didn't really understand the comparison you were trying to draw

Building CLI's should be seamless and intuitive

Assuming those issues get fixed, to enable this it'd just be a lot of inputs tsdx build --target node --format cjs --prod. That's kind of annoying to use and is why I'm not a huge fan of flag-based config -- config files are easier to manage and then you can share around config files such as a "CLI preset". It's not quite zero-config, but neither is 3 flags. Templates are comparatively only project init-time so the impact/usage is limited.

ivan-aksamentov commented 4 years ago

Make it possible to opt-out of dev/prod builds

Should be careful here. Many CLI projects would also ship a programmatic interface, that is the "classic" tsdx package + CLI, not just CLI.

For example, I want to ship bin/my-cli executable and lib/index.js module, and I would love to use tsdx right now, however:

right now it's a module only => cannot use it :red_circle:
okay, I can fiddle around and make it CLI-only => still cannot use it :red_circle:
if there would be a module + CLI setup => :heavy_check_mark:

If there would be a simple way to setup and build module+CLI packages, I am sure many lazy developers (like myself) would ship way more cool stuff. Right now the boilerplate barrier is too high.

agilgur5 commented 4 years ago

If there would be a simple way to setup and build module+CLI packages

@ivan-aksamentov Sounds like you're requesting multi-entry, i.e. #367

ivan-aksamentov commented 4 years ago

@agilgur5 Yes, multiple entries would be fantastic. For my case I'd like one entry to be an executable and another to be a library. That is, the CLI-ness and

Make it possible to opt-out of dev/prod builds

should be configurable on per-entry basis (and this is probably true for other config options as well).

This basically becomes similar to having two separate tsdx builds with two different configs, but in the same directory and with one config file. As an alternative to multi-entry, and a quick and dirty solution maybe tsdx config can export an array of config objects, similarly to how webpack config can (this sometimes used for SSR setups).

leodr commented 4 years ago

ncc's use-case is primarily serverless, though I've also seen it used in GH Actions (like in upstream @bahmutov/npm-install which I've been contributing to). CLIs don't have to be packaged into one file if they're installed with Node and ideally shouldn't be so you're not duplicating dependencies. TSDX itself isn't one file. Also to put everything into one file is just a configuration in Rollup, setting external to always return false / removing external as you've previously mentioned in #420 (comment) and I have in #179 (comment) .

I've run into an issue recently while developing a CLI similar to create-react-app, which uses Ink (needs the react package) to render it's console interface. Problem is, that my CLI is also supposed to bundle React code and I get an error if I don't bundle the dependencies together with the CLI, because 2 instances of the react module exist in my node_modules.

It's a boilerplate nightmare right now and I would definitely appreciate it if TSDX could take away some of that overhead.

agilgur5 commented 4 years ago

if I don't bundle the dependencies together with the CLI, because 2 instances of the react module exist in my node_modules.

That issue doesn't sound like it's specific or unique to CLIs. That also means you're duplicating React. Sounds like you have a versioning issue if your CLI and Ink aren't sharing the same React dependency, would check your yarn.lock. Difficult to tell more without the actual error or a repro, but this isn't the place for that in any case.

As I said, by bundling you're also duplicating deps in your users' node_modules on top of the dupes within your package. You also make it impossible for users to resolve security issues with your deps on their own (e.g. updating patch versions, Yarn resolutions, etc); you become an SPoF.

It's a boilerplate nightmare right now and I would definitely appreciate it if TSDX could take away some of that overhead.

I'm not sure why it's so bad for you, but as I mentioned, it is already doable to do bundling in TSDX if you really want/need to, and it is a one-liner. For CLIs though, the shebang issue linked currently requires a post-build step to remediate

leodr commented 4 years ago

I probably didn't describe it that well. What is meant is that when you install the CLI into your React project, there are possibly 2 versions of react. The one that your project uses to render and the one that my CLI uses to render its output. These have been clashing for me and I wasn't able to run the CLI.

agilgur5 commented 4 years ago

Ah you meant your consumer and your package. I'm assuming you also mean your package is a devDep of the consumer (vs globally installed CLI). That should also work the same though, if your package and their React versions are compatible there will only be one React in node_modules. If incompatible, it makes their node_modules have nesting, they have their React version and your package your React version. Can read more in this SO answer

That's normal, happens with some frequency with deps and shouldn't be an issue. Again you didn't specify an error or provide a repro so I don't know more details. But this seems to be off-topic.

leodr commented 4 years ago

Sorry if this is off-topic. I will further investigate my issue and will come back to this issue if I find out anything relevant.

agilgur5 commented 4 years ago

Closing this out as stale/duplicate since OP has not responded since opening and it is mostly duplicative of existing issues

jaredpalmer commented 4 years ago

Please re-open. This is still very much an issue.

agilgur5 commented 4 years ago

You're welcome to respond to previous comments. Otherwise, this issue is adding noise as a duplicate (and the comments are illustrative of that).

jaredpalmer / tsdx