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

Adding handling for multiple Webpack configs #335

Merged
merged 7 commits into from
May 17, 2016

Conversation

GreenGremlin
Copy link
Contributor

If the Webpack config is an array, then this change will use the first config that includes a resolve section.

@GreenGremlin
Copy link
Contributor Author

I'm have a bear of a time trying to figure out how to configure eslint-import-resolver-webpack as a dependency in package.json directly from github. Any ideas?

@benmosher
Copy link
Member

Code looks great, strategy makes sense. Can you add a note about this new behavior in the resolver README and the root change log?

As for installing from GitHub: not sure, since it's in a folder.

@GreenGremlin
Copy link
Contributor Author

Thanks! I'll add the note.

I ended up getting it to work like this:

"eslint-import-resolver-webpack": "file:node_modules/eslint-plugin-import/resolvers/webpack",
"eslint-plugin-import": "github:greengremlin/eslint-plugin-import#master-build",

@GreenGremlin
Copy link
Contributor Author

Done. I also added support for a config-index setting for manually specifying which config to load.

@GreenGremlin
Copy link
Contributor Author

The failure is a timeout in a test that does not cover my changes.

settings:
import/resolver:
webpack: { config: 'webpack.multiple.config.js' }
config-index: 1
Copy link
Member

Choose a reason for hiding this comment

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

This actually needs to be a sibling of the "config" key, I think. The settings that the resolver gets are the contents of the object keyed by "webpack"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I also fixed the JSON piece in the YAML example.

@benmosher
Copy link
Member

Added docs look great, thanks! Just one note on the config-index example.

And that test timeout thing is a real pain, I thought I had fixed it but apparently not.

@GreenGremlin
Copy link
Contributor Author

Done. I also fixed the mixed JSON / YAML in the documentation

@GreenGremlin
Copy link
Contributor Author

Is anything else needed to merge this?

@benmosher
Copy link
Member

Nope, looks good!

@benmosher benmosher merged commit c15901b into import-js:master May 17, 2016
@GreenGremlin
Copy link
Contributor Author

Great! Thanks. Any idea when you might push a release to npm?

@benmosher
Copy link
Member

Likely by Friday evening.

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