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

Production bundle #115

Closed
porfirioribeiro opened this issue Apr 7, 2018 · 13 comments
Closed

Production bundle #115

porfirioribeiro opened this issue Apr 7, 2018 · 13 comments
Labels
enhancement 🚀 has-fix A fix is available, but may not yet be released.

Comments

@porfirioribeiro
Copy link

Hi, would it be possible to add an option to use https://github.com/rollup/rollup-plugin-replace
To be able to make different builds for development and production

@Andarist
Copy link
Collaborator

Andarist commented Apr 7, 2018

Sure thing, but could you specify for which format u'd like to output those 2 builds? Seems only relevant for umd (and maybe cjs targeting node).

@porfirioribeiro
Copy link
Author

Well my idea is to make my library provide both production and development builds

if (process.env.NODE_ENV === 'production') {
  module.exports = require('./cjs/react.production.min.js');
} else {
  module.exports = require('./cjs/react.development.js');
}

So i can add useful debug code for development but also provide the smallest code in production

@Andarist
Copy link
Collaborator

Andarist commented Apr 8, 2018

is your library targeting node, browsers or both?

@porfirioribeiro
Copy link
Author

porfirioribeiro commented Apr 8, 2018 via email

@Andarist
Copy link
Collaborator

Andarist commented Apr 8, 2018

For browsers u dont need this. Just use process.env.NODE_ENV checks, do not replace them before publishing and let consumers' bundlers do that.

Example in the wild can be check here.

@porfirioribeiro
Copy link
Author

Ok, i get it, will use that for now!

@stereobooster
Copy link

The same #181 ?

@danielkcz
Copy link

danielkcz commented Nov 15, 2019

Just referencing one issue we got reported over at mobx: mobxjs/mobx-react#801

@Andarist I tend to agree that this should be indeed a concern for consumer bundlers. What about UMD? There is no bundling happening, it's used directly and it's unlikely user will be mocking process out.

Furthermore, when I look for inspiration to React package, they indeed are keeping env checks in development bundles, but removing (evaluating) them for a production bundle:

https://unpkg.com/browse/[email protected]/cjs/react.development.js
https://unpkg.com/browse/[email protected]/cjs/react.production.min.js

The TSDX went a slightly different route, they are keeping ESM bundle with those checks because it's a good assumption that one will be bundled by a consumer. But UMD is stripped down. For CJS the approach from #115 (comment) works very well. Have a look at Formik for example.

https://unpkg.com/browse/[email protected]/dist/

@Andarist
Copy link
Collaborator

What about UMD?

Definitely should be replaced, UMDs are intended to be used "as is".

Furthermore, when I look for inspiration to React package, they indeed are keeping env checks in development bundles, but removing (evaluating) them for a production bundle:

Keep in mind that actually this is React's entry point - https://unpkg.com/[email protected]/index.js . This is a proxy file to prod/dev bundles (similar strategy is used by https://preconstruct.tools/ ). It's written like this because process.env is supposedly slow in node, so better to just read it one time rather than all over the place. Also that dev bundle has actually replaced all process.env.NODE_ENV with "development" - it just wraps its content with one more non-replaced env check. Not sure why this would be needed? Maybe just some extra measure to be sure that nobody ships/executes two copies of React.

I believe a similar strategy (proxy file) could be used for CJS entries in microbundle

@danielkcz
Copy link

danielkcz commented Nov 15, 2019

Yea, so ultimately I believe the TSDX has chosen the right path and it would be nice if microbundle could follow :)

Keep in mind that actually this is React's entry point

Yea, but their development bundle still refers to process.

@developit developit added the has-fix A fix is available, but may not yet be released. label Dec 18, 2020
@AdrianMrn
Copy link

Heya, I'm getting this in a UMD build, any idea what the fix was @developit ?

@developit
Copy link
Owner

I don't recall. If you're trying to use env checks in UMD that's not recommended. If you're trying to use the React package and seeing this in bundles, try passing --define process.env.NODE_ENV=production

@AdrianMrn
Copy link

Thanks, --define process.env.NODE_ENV=production fixed it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🚀 has-fix A fix is available, but may not yet be released.
Projects
None yet
Development

No branches or pull requests

6 participants