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

add-missing-env-variable-in-webpack-config #1044

Merged

Conversation

kamalbennani
Copy link
Contributor

@kamalbennani kamalbennani commented Mar 12, 2018

Following #938, I am facing a problem where my project is composed of a several applications and each them might define a "custom" webpack configuration which extends a common configuration which is defined in the root of the project.

/
apps/
- app1/
  webpackConfig.js  <-- specific to app1
  .eslintrc
- app2/
  webpackConfig.js  <-- specific to app2
  .eslintrc
- app3/
  webpackConfig.js <-- specific to app3
  .eslintrc
webpack.config.js <-- common configuration

A custom webpack configuration looks like so:
apps/app1/webpackConfig.js

export function (baseConfig) => {
 newConfig = baseConfig
 // Some custom manipulation
 ....
 return newConfig
}

Note that the common webpack configuration is responsible for determining which webpack configuration to load based on the application name.
So for instance, to run the app1, we execute the following command :

webpack-dev-server  --config webpack.config.js --env app1

The problem is that the 'env' variable is being ignored, So I can't do something like that:
apps/app1/.eslintrc

const path = require('path');
module.exports = {
  settings: {
    'import/resolver': {
      webpack: {
        config: path.resolve(__dirname, '../../webpack.config.js'),
        env: 'app1'
      }
    }
  },
};

The solution is to grab the env from the settings and to pass it to the webpackConfig in the webpack resolver:
resolvers/webpack/index.js

var env = get(settings, 'env')
if (typeof webpackConfig === 'function') {
   webpackConfig = webpackConfig(env)
}

@coveralls
Copy link

coveralls commented Mar 12, 2018

Coverage Status

Coverage increased (+0.002%) to 96.236% when pulling 0844645 on botify-labs:add-missing-env-variable-in-webpack-config into 158f4e8 on benmosher:master.

@coveralls
Copy link

coveralls commented Mar 12, 2018

Coverage Status

Coverage increased (+0.002%) to 96.236% when pulling 4b311ac on botify-labs:add-missing-env-variable-in-webpack-config into 158f4e8 on benmosher:master.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a good fix; is there any way to provide a test to cover it?

@kamalbennani
Copy link
Contributor Author

Here you are, thanks for accepting the PR 👍

@ljharb ljharb merged commit 5f7ecd3 into import-js:master Mar 14, 2018
@kamalbennani kamalbennani deleted the add-missing-env-variable-in-webpack-config branch March 15, 2018 10:53
@kamalbennani
Copy link
Contributor Author

kamalbennani commented Mar 15, 2018

@ljharb Can you tell me when are you planning to release the resolver-webpack, because as you might now I can't target the commit so I need to wait for the release.
By the way thanks for the merge

@ljharb
Copy link
Member

ljharb commented Apr 11, 2018

@kamalbennani v0.9.0 was published.

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

Successfully merging this pull request may close these issues.

3 participants