False warning message starting from version 5.15.3 (you are running a minified build, but 'process.end.NODE_ENV'...)

[ranyitz]

• Issue:
  • Did you check this issue wasn't filed before?
  • Elaborate on your issue. What behavior did you expect?
  • State the versions of MobX and relevant libraries. Which browser / node / ... version?

The Problem

On version [email protected] the bundle mobx.module.js throws an error which is a false positive. It says that the bundler was not configured for production which in fact it was.

[mobx] you are running a minified build, but 'process.end.NODE_ENV' was not set to 'production' in your bundler. This results in an unnecessarily large and slow bundle

This is a degradation from [email protected] where there was no such message.

Explanation

Digging into the build process of mobx i've found this change which did the following replacement

- process.env.NODE_ENV !== "production"
+ "development" !== "production"

mobx/src/v5/mobx.ts

Line 42 in d5488a1

process.env.NODE_ENV !== "production" &&

It makes mobx send this message in case the code passed minification regardless of the process.env.NODE_ENV that was used during build time. Sending a false message to the user.

Possible Solution

While the replacement is good for optimizing production bundles, for development it is not critical. Removing this line should remove this warning. If we would like to optimize the development bundle we can later on see how we can do it without sending the false warning.

[danielkcz]

Related to #2264

Thanks for the report. Can you please describe your environment?

This has happened mostly by accident when merging a code base (#2248). But the intention is actually good. The process shouldn't be present in commonjs builds at all.

That is true for mobx.min.js as production build with correctly eliminated dead code. However, the package.json does not refer to this minified file. The correct fix I see here is to change that so a minified bundle is used by default.

[mweststrate]

@FredyC, see also my comment on the PR, using the .min. by default just exposes the opposite problem: getting minified code while devving that lacks the assertions that are especially build for dev I? I think we should stick to the old setup, where you get the 'dev' version by default, unless you configure your bundler to substitute for prod, and make sure nothing dies if there is no substitution at all

[danielkcz]

Yea, I think we will go the way TSDX does, by exposing index.js which looks like this (plus safety checks with typeof).

if (process.env.NODE_ENV === 'production') {
  module.exports = require('./mobx.min.js')
} else {
  module.exports = require('./mobx.js')
}

This way the correct bundle will be selected depending if you are developing or bundling for a production.

[ranyitz]

@FredyC We are using [email protected] for bundling.

The problem is in the published file mobx.module.js

When opening its contents, you'll see that there is a "development" !== "production" condition which is always true. If you'll create two bundles (just like react) it wont necessarily solve the problem above. I think that the code that handles the testCodeMinification check should leave the process.env.NODE_ENV for the bundler for the check to work properly instead of replacing it in its own build.

[danielkcz]

Ah yes, you are right, module bundle should be left untouched because it is handled by the bundler. That's definitely a mistake I will fix.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.