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

Recommended a method of handling Webpack function config arguments #938

Closed
wants to merge 1 commit into from
Closed

Conversation

robwierzbowski
Copy link

@robwierzbowski robwierzbowski commented Oct 5, 2017

I ran into the undefined env issue with webpack function configuration, and noticed while searching that plenty of others have had the same problem. Example issues:

#398
AtomLinter/linter-eslint#625

The Vue and React boilerplates I've used recently both use function configuration, so I think this is probably a common issue. Adding a recommended way of handling this issue to the documentation could be helpful for webpack users, and save them the time I spent looking for the cause of the problem and a solution.

Here's a draft of what I believe is the simplest way to handle the issue. Feedback welcome; I'm happy to revise.

I ran into the undefined `env` issue with webpack function configuration, and
noticed while searching that plenty of others have had the same problem. Some
related issues:

#398
AtomLinter/linter-eslint#625
@@ -336,6 +336,19 @@ settings:
[`eslint_d`]: https://www.npmjs.com/package/eslint_d
[`eslint-loader`]: https://www.npmjs.com/package/eslint-loader

## Webpack configuration arguments

This plugin doesn't pass arguments to [Webpack configuration functions](https://webpack.js.org/configuration/configuration-types/#exporting-a-function). If a Webpack configuration function uses the `env` or `argv` arguments, values must be supplied to avoid `argument undefined` errors. On Node 6.4 or greater we recommend [default parameters](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Default_parameters):
Copy link
Member

Choose a reason for hiding this comment

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

is this something that the plugin could fix?

Copy link
Author

@robwierzbowski robwierzbowski Oct 5, 2017

Choose a reason for hiding this comment

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

Possibly partially.

Case A: Use during compilation

Since webpack and eslint are running in the same env, it could be possible. The relevant webpack function call is here: https://github.com/benmosher/eslint-plugin-import/blob/a69b77181c423043d39f1c8752b296aaa95facfc/resolvers/webpack/index.js#L84-L86

Case B: Use in the code editor

I don't think this case is solvable. There's no way for the editor to know what env you're running your build process in.

Copy link
Author

@robwierzbowski robwierzbowski Oct 5, 2017

Choose a reason for hiding this comment

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

The plugin could expose a command line or config option to pass args to the webpack function. IMO, people have different enough needs that handling in webpack config on a case by case basis is probably best.

I could see providing a default env.NODE_ENV in the plugin, to automatically fix the most common case. That would be prescriptive and could cause unexpected behavior, but might be a good "works out of the box" solution.

@robwierzbowski
Copy link
Author

robwierzbowski commented Oct 6, 2017

After thinking about this a bit more, maybe we can solve it mostly in the plugin. Tell me what you think and I'll edit this PR.

For ESLint as loader, pass in process.env. If possible pass in any args passed to webpack — not sure if they’re available in plugins, but maybe?

For ESLint as an editor package, what do you think about making the executive decision to pass in a stub/fake env object that sets NODE_ENV: development? The issues I saw posted were all for the NODE_ENV prop, so it might be the most common case. For argv, don’t pass anything; I don't think there's any way to guess what a person might set.

Finally, I can revise the doc addition to say “if you reference any env or argv values in your Webpack config, please stub them for the code editor. The package stubs NODE_ENV: development as a convenience.”

Thoughts?

@ljharb
Copy link
Member

ljharb commented Oct 6, 2017

The way editors should be handled is: if they differ from the command line in any way, they're broken. In other words, pretend editors don't exist.

NODE_ENV can certainly be defaulted to development, yes.

@robwierzbowski
Copy link
Author

OK, I'll revise this PR, see if I can pass through webpack args, etc.

@robwierzbowski
Copy link
Author

I'm closing this as I don't really need the functionality anymore and don't think I will complete this PR.

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

Successfully merging this pull request may close these issues.

2 participants