mobxjs / mobx-react-devtools

[DEPRECATED] Tools to perform runtime analyses of React applications powered by MobX and React
MIT License
1.23k stars 49 forks source link

Do nothing if NODE_ENV === 'production' #40

Closed mohsen1 closed 7 years ago

mohsen1 commented 7 years ago

Currently we have to check for NODE_ENV to include/not include the component. Shouldn't this component render null if NODE_ENV is production?

andykog commented 7 years ago

@mohsen1, checking for NODE_ENV by yourself is good way to go, you can make conditional import:

let DevTool;
if (if MY_ENV_VAR === 'development') {
  DevTool = require('mobx-react-devtools').default;
} else {
  DevTool = () => null;
}

then, assuming that you provided MY_ENV_VAR with webpack.ProvidePlugin and using uglifyjs, the whole module will be skipped so there is no superfluous code in production bundle. I don't think the same result is possible to achieve by checking NODE_ENV in mobx-react-devtool packgage as it is being pre-compiled in a single file.

andykog commented 7 years ago

Closing for inactivity, feel free to reopen

KaySackey commented 7 years ago

the same result is possible to achieve by checking NODE_ENV in mobx-react-devtool packgage as it is being pre-compiled in a single file.

If you're using it via Webpack 2, uglify will operate on everything including files that come out of node_modules. I know that it strips down React.

mohsen1 commented 7 years ago

@KaySackey Webpack does not exclude packages from node_modules by default. You have to use externals option: https://webpack.js.org/configuration/externals/

KaySackey commented 7 years ago

I know. I'm commenting because @andykog questioned if Uglify would work on items that are pre-compiled into a single file. The answer seems to be "yes".

andykog commented 7 years ago

@KaySackey, the original proposition was to simply return null in prod environment. uglify.js (as well as webpack 2 with its tree-shaking) won't prevent including superfluous code even if we wrap the content of entry point file in if (process.env.NODE_ENV !== 'production') {} (see example below). We have to wrap the whole bundle, generated by weback in this if() {} to make it work. Then we have to export some stubs so it won't break in production when using something like:

import DevTool, { configureDevtool } from 'mobx-react-devtools';
configureDevtool({});
render(<div><DevTool /></div>);

In theory this can be done, but seems a bit unnatural to me.

// That won't help:
(function() {
  modules = [
    (function() {
       if (process.env.NODE_ENV !== 'production') {
         var devtoolsHelpers = __webpack_require__(1);
         var devtoolsReactComponent = __webpack_require__(2);
         __webpack_require__(2); __webpack_require__(3); __webpack_require__(4); __webpack_require__(5);
       }
    }()),
    (function() { /* code */ }()),
    (function() { /* code */ }()),
    (function() { /* code */ }()),
    (function() { /* code */ }()),
    (function() { /* code */ }()),
  ]
  function __webpack_require__(n) {}
  /* other webpack helpers */
}());

How can uglify or tree-shaking help with this?

KaySackey commented 7 years ago

Breaking it down, I got it to work with a one line import

exports.DevTools = (process.env.NODE_ENV === 'production') ? () => <div></div> :  require('mobx-react-devtools').default;

In webpack, I add the DefinePlugin to force process.env.NODE_ENV into a constant value of 'production'

              new webpack.DefinePlugin({
                  'process.env': {NODE_ENV: JSON.stringify(nodeEnv)}
              }),

This means that Uglify sees the equivalent of:

exports.DevTool = (true) ? () => <div></div> :  require('mobx-react-devtools');

Then evaluates it to drop the value that will never be used---which is the entire import and assignment statement given pushed out by Webpack after it converts require('mobx-react-devtools').

The trick is:

  1. Webpack can understand CJS and require-imports.
  2. This isn't tree-shaking dependencies, its dropping a dead-code branch which is something Uglify can do.

Combine 1 and 2, and there you have it... the result is a CJS module without DevTools in production, and no re-writing of the code.

If it were added to the codebase here, it'd just be a separate index file that imports the compiled code.

Disclaimer: Sadly, this trickery won't work with rollup. But rollup has a plugin to convert commonjs modules to ES6. I've never tried it on DevTools on its own though.