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

DefinePlugin fails to inject process.env.NODE_ENV into React without UglifyJSPlugin #868

Closed
skozin opened this Issue Mar 9, 2015 · 7 comments

Comments

Projects
None yet
4 participants
@skozin

skozin commented Mar 9, 2015

package.json:

{
  "name": "test",
  "version": "0.0.1",
  "dependencies": {},
  "devDependencies": {
    "react": "0.12.2",
    "webpack": "1.7.2"
  }
}

main.js:

var React = require('react')
console.log(process.env.NODE_ENV)

webpack.config.js:

var webpack = require('webpack')

module.exports =  {
  entry: {
    app: './main'
  },
  output: {
    path: '_dist',
    filename: 'out.js'
  },
  resolve: {
    extensions: ['', '.js', '.json']
  },
  plugins: [
    new webpack.DefinePlugin({
      'process.env': {
        'NODE_ENV': '"production"'
      }
    })
  ]
}

Place all these files into the same directory and then run:

npm install
rm -rf _dist && ./node_modules/.bin/webpack

Then inspect _dist/out.js. You will see that process.env.NODE_ENV has been successfully injeted to main.js module (line 47):

var React = __webpack_require__(1)
console.log(("production"))

But all entries of process.env.NODE_ENV inside React modules remain intact, e.g. line 314:

("production" !== process.env.NODE_ENV ? warning(
  standardName == null,
  'Unknown DOM property ' + name + '. Did you mean ' + standardName + '?'
) : null);

You can verify this by searching for ("production") and process.env.NODE_ENV. Changing definition to a flat one doesn't help:

  plugins: [
    new webpack.DefinePlugin({
      'process.env.NODE_ENV': '"production"'
    })
  ]

However, when I add UglifyJSPlugin, injection starts working as intended: UglifyJS prints a lot of Condition always false messages, and there are no occurrences of NODE_ENV inside the minified out.js.

@skozin skozin changed the title from DefinePlugin fails to inject process.env.NODE_ENV into React to DefinePlugin fails to inject process.env.NODE_ENV into React without UglifyJSPlugin Mar 9, 2015

@sokra

This comment has been minimized.

Member

sokra commented Mar 9, 2015

when you take a closer look to the react file you'll notice that it looks like this:

...
if ("production" !== process.env.NODE_ENV) {
...
    ("production" !== process.env.NODE_ENV ? warning(
      standardName == null,
      'Unknown DOM property ' + name + '. Did you mean ' + standardName + '?'
    ) : null);
...
}
...

webpack already replaces the top-most if ("production" !== process.env.NODE_ENV) with if (false) and skips the block. So the process.env.NODE_ENV in the block never gets executed (UglifyJs will remove it, dead code).

...
if (false) {
...
    ("production" !== process.env.NODE_ENV ? warning(
      standardName == null,
      'Unknown DOM property ' + name + '. Did you mean ' + standardName + '?'
    ) : null);
...
}
...
@skozin

This comment has been minimized.

skozin commented Mar 9, 2015

You're right, I should have inspected the output more carefully. Thanks for the quick response and sorry for the bother!

@skozin skozin closed this Mar 9, 2015

@skozin

This comment has been minimized.

skozin commented Mar 9, 2015

Am I understanding correctly that Webpack is so clever that it detects the dead code and doesn't perform a no-op injection?

@jhnns

This comment has been minimized.

Member

jhnns commented Mar 11, 2015

The DefinePlugin just replaces occurrences of the given identifiers with the given expressions. After that, UglifyJS detects dead code blocks and removes them.

@skozin

This comment has been minimized.

skozin commented Mar 11, 2015

@jhnns, then I don't understand why it didn't replace the inner process.env.NODE_ENV expression, inside the if (false) block (see @sokra's comment)...

@loganfsmyth

This comment has been minimized.

Contributor

loganfsmyth commented Mar 11, 2015

@skozin Webpack does all of its AST walking and transforming together on an AST, and it does so while performing static analysis, so if it recognizes that it can't get into one side of an if, it won't run any transforms in that part. DefinePlugin hooks into that transform/traversal so it won't bother substituting stuff into the if(false){ side of the if.

@skozin

This comment has been minimized.

skozin commented Mar 11, 2015

@loganfsmyth, thanks! That's a great feature :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment