feat: support passing config to generateWebpackConfig for merging#343
feat: support passing config to generateWebpackConfig for merging#343justin808 merged 8 commits intoshakacode:masterfrom G-Rath:webpack-merge-param
generateWebpackConfig for merging#343Conversation
There was a problem hiding this comment.
It's a good proposal with a clean implementation.
It would be good to update the current dummy app based on these changes.
✅ Added tests
✅ Updated Documentation
🚫 Updated Changelog
Please update the changelog.
@justin808 Should we prepare for 7.1.0?
ahangarha
left a comment
ahangarha
left a comment
There was a problem hiding this comment.
Thanks for applying the changes. It looks good.
We just have an issue with passing multiple configs. Check my comment.
|
Thanks for the reviews - I'll action them in the next couple of weeks as I'm working on #349 right now |
|
@G-Rath Ping me when ready for review! |
| // having a new object is essential. | ||
| module.exports = merge({}, baseWebpackConfig, options) | ||
| // This results in a new object copied from the mutable global | ||
| module.exports = generateWebpackConfig(options) |
There was a problem hiding this comment.
- It's not clear where to place this code. What file is this?
- With the new object rather than global, some people might get confused if they are migrating old code of multiple files that simply mutated the global object.
There was a problem hiding this comment.
These changes might be fine as-is...just some thoughts.
There was a problem hiding this comment.
@justin808 I've reworded the documentation to make it clearer where the file lives. I'm not sure what you mean by your second point though?
|
Yup go for it! It would also be good to get released :) |
3 participants
Summary
Enables providing
generateWebpackConfigan optional object that is included in the config merge, reducing the amount of boilerplate code you have to write when doing basic customization to the config.Pull Request checklist
Other Information
Resolves #329