jaredpalmer / tsdx

Zero-config CLI for TypeScript package development
https://tsdx.io
MIT License
11.26k stars 508 forks source link

Using Babel NODE_ENV for separate dev & prod plugins #687

Open cvlmtg opened 4 years ago

cvlmtg commented 4 years ago

Current Behavior

I'd like to use babel-plugin-transform-react-remove-prop-types to remove prop-types from production build, while leaving them in the development build. I've added this to my .babelrc.js

module.exports = {
  env: {
    production: {
      plugins: [
        [
          'transform-react-remove-prop-types',
          {
            removeImport: true,
            mode: 'remove'
          }
        ]
      ]
    }
  },
  etc..

However I see that tsdx produces its .cjs.production.min.js without setting NODE_ENV, so I can apply my trasformation either on both builds or none of them...

Desired Behavior

The ability to have different transformations applied on development / production build. NODE_ENV is the standard way AFAIK, but if there's another way to obtain the same results I'll be happy to follow it.

Thanks!

agilgur5 commented 4 years ago

So this is a valid use-case, but there's sort of an issue with it when it comes to the ESM build -- there's only one ESM build (though maybe we'll find a workaround for this soon since React is currently facing the same issue). So I'm not really sure what it should do in that case.

but if there's another way to obtain the same results I'll be happy to follow it.

Indeed there is -- put your propTypes definition behind a if (process.env.NODE_ENV === 'development') check, i.e.:

export function MyComponent ({ props }) {
  // ...
}

if (process.env.NODE_ENV === 'development') {
  MyComponent.propTypes = {
    // ...
  }
}

Per the docs, the prod CJS version will strip it out entirely. It also works for the ESM build too, which relies on your bundler & minifier to strip it out.

cvlmtg commented 4 years ago

@agilgur5 thanks for the answer!

Regarding the ESM build, unless I misunderstood its purpose, I'd treat it as a development plugin. Since I read it's meant to be minified or otherwise processed by a consumer, I think it's fine to build it without stripping anything. If the consumer of the bundle wants to strip it, it can still use babel-plugin-transform-react-remove-prop-types.

As for the workaround, thanks! But the production bundle still has require("prop-types") which means that a bundler will still try to bundle it. My goal was to avoid imposing my library's users to add 'prop-types' as a dependency, especially if it's not used.

Anyway, it's not possible to use import inside an if, so I tried to use require() and ignore eslint, but tsc now gives an error (TS7034).

agilgur5 commented 4 years ago

Regarding the ESM build, unless I misunderstood its purpose, I'd treat it as a development plugin.

The ESM build isn't a development build and it'll probably be used by consumers of your library that use Webpack. We just can't make separate dev/prod bundles of ESM, so we expect a bundler to handle that (and basically only bundlers read package.json module).

If the consumer of the bundle wants to strip it, it can still use babel-plugin-transform-react-remove-prop-types.

So there's some issues with this approach:

  1. It creates non-uniformity between ESM and CJS builds
  2. It requires the user installing the plugin of course, and also running Babel on their node_modules. I think CRA does this now by default(?), but generally this was specifically avoided in Webpack configs due to how much time it would add to builds. As I recall CRA changed their stance because some libraries started shipping ESM only builds (or rather, not built at all, just source ESM).

Anyway, it's not possible to use import inside an if, so I tried to use require() and ignore eslint, but tsc now gives an error (TS7034).

Not sure I understand why that would give you a type error if import didn't? You could also dynamic import() in an IIFE, and I think that'd result in Rollup code-splitting a chunk out too, but not 100% sure. There might be some transpilation ramifications to using dynamic import() though.

cvlmtg commented 4 years ago

Hi @agilgur5, sorry for replying so late. Regarding the ESM module, I just tried to be helpful, but given my limited experience with bundlers and modules, feel free to ignore my suggestion :)