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

RFC: Better DX for making CLI #793

Closed
jaredpalmer opened this issue Aug 10, 2020 · 12 comments
Closed

RFC: Better DX for making CLI #793

jaredpalmer opened this issue Aug 10, 2020 · 12 comments
Labels
problem: stale Issue has not been responded to in some time solution: duplicate This issue or pull request already exists

Comments

@jaredpalmer
Copy link
Owner

jaredpalmer commented Aug 10, 2020

Current Behavior

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

  • The preserve-shebang plugin doesn't actually work (Issue with node shebang #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

Suggested Solution

  • Make it possible to opt-out of dev/prod builds
  • 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
  • Create a CLI template
    • Explore Gluegun's test setup

Who does this impact? Who is this for?

  • Me, advanced users

Describe alternatives you've considered

Additional context

@agilgur5
Copy link
Collaborator

agilgur5 commented Aug 14, 2020

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 #351 (comment), 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.

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) .

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
Copy link

ivan-aksamentov commented Aug 16, 2020

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 🔴

  • okay, I can fiddle around and make it CLI-only => still cannot use it 🔴

  • if there would be a module + CLI setup => ✔️

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
Copy link
Collaborator

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
Copy link

ivan-aksamentov commented Aug 23, 2020

@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).

@agilgur5 agilgur5 added the problem: stale Issue has not been responded to in some time label Sep 1, 2020
@leodr

This comment has been minimized.

@agilgur5

This comment has been minimized.

@leodr

This comment has been minimized.

@agilgur5

This comment has been minimized.

@leodr

This comment has been minimized.

@agilgur5
Copy link
Collaborator

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

@agilgur5 agilgur5 added the solution: duplicate This issue or pull request already exists label Sep 14, 2020
@jaredpalmer
Copy link
Owner Author

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

@agilgur5
Copy link
Collaborator

agilgur5 commented Sep 14, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
problem: stale Issue has not been responded to in some time solution: duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

4 participants