vadimdemedes / ink

🌈 React for interactive command-line apps
https://term.ink
MIT License
27.23k stars 611 forks source link

Bundle ink without `devtools` #571

Open fantua opened 1 year ago

fantua commented 1 year ago

I would like to bundle ink with esbuild using the following config:

bundle: true,
define: {
  'process.env.DEV': 'false'
},
platform: 'node',
format: 'esm'

'process.env.DEV': 'false' should do dead code elimination for this part: https://github.com/vadimdemedes/ink/blob/fb66872f903311d5b78ba113d009fe8e6f853e51/src/reconciler.ts#L26-L44

But devtools import still presents because we’re not using global process reference. Once I remove this line everything works as expected: https://github.com/vadimdemedes/ink/blob/fb66872f903311d5b78ba113d009fe8e6f853e51/src/reconciler.ts#L1

see https://github.com/evanw/esbuild/issues/3015 for more details

vadimdemedes commented 1 year ago

What is the benefit of excluding devtools.js from the resulting bundle?

sindresorhus commented 1 year ago

But devtools import still presents because we’re not using global process reference. Once I remove this line everything works as expected:

This sounds like something esbuild should fix. If they support handling the global process, they should handle the imported one. Open an issue on esbuild instead.

fantua commented 1 year ago

First of all devtools is useful for dev mode only, I don't need it in production bundle.

Benefit of excluding devtools.js from the resulting bundle:

fantua commented 1 year ago

This sounds like something esbuild should fix. If they support handling the global process, they should handle the imported one. Open an issue on esbuild instead.

Issue description contains a link to esbuild issue alteady. Basically esbuild supports global variables only https://github.com/evanw/esbuild/issues/3015#issuecomment-1479955689.

vadimdemedes commented 1 year ago

First of all devtools is useful for dev mode only, I don't need it in production bundle.

react-devtools-core isn't included with Ink and it's an optional peer dependency, so it shouldn't be part of your bundle if you didn't install it yourself.

fix for the following error (I don't install react-devtools-core as a dependency):

esbuild is suggesting a fix there to mark react-devtools-core as an external dependency. Try that.

This sounds like something esbuild should fix. If they support handling the global process, they should handle the imported one. Open an issue on esbuild instead.

Agree with @sindresorhus on this one. If Node.js recommends to import process now, it makes sense for bundlers to handle that scenario.

eXhumer commented 1 year ago

I came across this issue while trying to bundle my scripts with webpack to package into an executable.

I found that replacing

https://github.com/vadimdemedes/ink/blob/8a0476024cf997e0dbc02e493ad3972349b02474/src/reconciler.ts#L26

https://github.com/vadimdemedes/ink/blob/8a0476024cf997e0dbc02e493ad3972349b02474/src/ink.tsx#L98

with if (process.env['NODE_ENV'] === 'development') { fixes the issue for me with webpack. webpack correctly checks the node environment and ignores the devtools.js import and related imports in production builds.

baublet commented 8 months ago

The way the dev tools are initialized (as a top-level await behind an if statement and shipped in with the build you're shipping to users) makes this library difficult to bundle and might be hindering adoption. As is, we need to utilize one of the heftier static analysis tools that can support understanding and removing the dead code if we want to use this in CJS code. Normal ESM/CJS problems, so might not be high on the priority list.

But at a glance, the entire library appears to be sync code except for where we're initializing the dev tools behind an if statement... it almost looks like a mistake that somehow made it into the prod builds.

The fix seems easy, though? Don't ship your dev tools to your users. Have a shim that initializes them before you import the rest of your (CJS) code and run that in development?

For what it's worth, though, thanks for the cool library, and hoping I don't have to switch off it!

strogonoff commented 5 days ago

esbuild is suggesting a fix there to mark react-devtools-core as an external dependency. Try that.

esbuild’s advice is predicated on react-devtools-core being delivered in some other way. The bundle with Ink still invokes react-devtools-core at runtime, which therefore breaks if one attempts this “fix”.

Since bundling devtools doesn’t exactly make sense for production builds, we have to perform string replacement on JS bundles with Ink to comment out react-devtools-core…