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

Using Babel NODE_ENV for separate dev & prod plugins #687

Open
cvlmtg opened this issue Apr 18, 2020 · 4 comments
Open

Using Babel NODE_ENV for separate dev & prod plugins #687

cvlmtg opened this issue Apr 18, 2020 · 4 comments
Labels
kind: feature New feature or request solution: workaround available There is a workaround available for this issue

Comments

@cvlmtg
Copy link

cvlmtg commented Apr 18, 2020

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 agilgur5 changed the title Question: usage of NODE_ENV Using Babel NODE_ENV for separate dev & prod plugins Apr 18, 2020
@agilgur5 agilgur5 added the kind: feature New feature or request label Apr 18, 2020
@agilgur5
Copy link
Collaborator

agilgur5 commented Apr 18, 2020

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.

@agilgur5 agilgur5 added the solution: workaround available There is a workaround available for this issue label Apr 18, 2020
@cvlmtg
Copy link
Author

cvlmtg commented Apr 18, 2020

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

agilgur5 commented Apr 21, 2020

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.

@agilgur5 agilgur5 added the problem: stale Issue has not been responded to in some time label Apr 26, 2020
@cvlmtg
Copy link
Author

cvlmtg commented Apr 26, 2020

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

@agilgur5 agilgur5 removed the problem: stale Issue has not been responded to in some time label Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: feature New feature or request solution: workaround available There is a workaround available for this issue
Projects
None yet
Development

No branches or pull requests

2 participants