[GitHub] williaster commented on a change in pull request #5786: Configure webpack-dev-server

[GitBox <gi...@apache.org>]

williaster commented on a change in pull request #5786: Configure webpack-dev-server
URL: https://github.com/apache/incubator-superset/pull/5786#discussion_r214475864
 
 

 ##########
 File path: superset/assets/webpack.config.js
 ##########
 @@ -11,6 +14,54 @@ const BUILD_DIR = path.resolve(__dirname, './dist');
 
 const isDevMode = process.env.NODE_ENV !== 'production';
 
 Review comment:
   one comment here is that I may have broken this flag when moving to webpack 4 which introduced the concept of `mode=production/development`. I thought that also set a `NODE_ENV` (because webpack 4 was supposed to be all about smart defaults) but it seems like that [might not be true](https://github.com/webpack/webpack/issues/6460).
   
   So I think we either need to 
   - reference the mode variable (ie `argv.mode === 'production'`)
   - explicitly define the `NODE_ENV` in our npm webpack scripts (`NODE_ENV=production webpack ...`)
   
   I can fix this in a separate PR as well if you prefer because it might have implications for the other logic in this file that relies on `isDevMode` (which I think is always true right now)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org