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

DEV code is now in production bundle in master #2244

Closed
gaearon opened this issue Feb 16, 2017 · 6 comments
Closed

DEV code is now in production bundle in master #2244

gaearon opened this issue Feb 16, 2017 · 6 comments
Assignees
Labels

Comments

@gaearon
Copy link
Contributor

gaearon commented Feb 16, 2017

This PR broke Uglify's dead code elimination for DEV: #2030.

Uglify is not smart enough to understand that if a literal is assigned to a variable, and later that variable gets compared, it's the same as literal. As a result, DEV-only code ends up in the production bundle:

{var c=t[r];"production"!==p&&void 0===e[c]&&(0,l["default"])('No reducer provided for key "'+c+'"'),"function"==typeof e[c

I suggest reverting #2030, at least in the current form.

@timdorr
Copy link
Member

timdorr commented Feb 16, 2017

Yeah, not sure why I didn't give that more scrutiny. Luckily, it hasn't been released.

We may want to add a quick check of the gzipped UMD bundle size to the build scripts. We do this on react router and it helps guard against ballooning the bundle size, provided you know what a "good" size looks like.

I'll revert this once I'm on desktop (since it can't be done automatically).

@gaearon
Copy link
Contributor Author

gaearon commented Feb 16, 2017

We may want to add a quick check of the gzipped UMD bundle size to the build scripts.

Yes, that would be great!

@Andarist
Copy link
Contributor

Do you guys know by any chance how this can be avoided?

@timdorr
Copy link
Member

timdorr commented Mar 15, 2017

That sounds like a tooling problem. We don't add the polyfill ourselves, so it would seem you need to configure your tooling not to do that. It's similar to checking for a DOM with typeof window !== 'undefined'.

@Andarist
Copy link
Contributor

Andarist commented Mar 19, 2017

yeah, u have reverted your checks to if (process.env.NODE_ENV !== 'production') {, but the mentioned commit tried not to only cache this value, but also guarded against lack of process like this:
var NODE_ENV = typeof process !== 'undefined' ? process.env.NODE_ENV : 'development'
this has triggered (on your previous webpack config too, i see u have switched to rollup now) process polyfill to be included

anyhow i'll try to explore possibilities to avoid this, from that I understand Aurelia has its own bundler which does not recognize process and packages that use it cannot be properly transpiled by its users

EDIT://

also have checked the mentioned commit, but bundled with rollup - bundled file does not include this polyfill (probably thanks to tree shaking?) but is leaving this in the code:

var NODE_ENV = typeof process !== 'undefined' ? "development" : 'development';

so not rly a good solution either, as it would always fallback to 'development'

also out of curiosity - what were the reasons behind chosing rollup vs webpack2?

@timdorr
Copy link
Member

timdorr commented Mar 19, 2017

@Andarist It's 50% smaller: #2283 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants