-
-
Notifications
You must be signed in to change notification settings - Fork 622
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
cli: fix watch options for array config #892
Conversation
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
Please sign the CLA :) |
@anshumanv per the CLA assistant page I have signed it (I did so as soon as I opened the PR), here's a screenshot of what I see when I visit the link from here (with my email obfuscated): Any tips on what I can do to reset and re-sign so that it shows correctly are appreciated! Update: |
Thanks @chaseadamsio, can you try closing and reopening this PR? |
Issue #4594 points out that watch options aren't being honored when the Webpack config is an Array. In cases when it's an array, there may not be a firstOptions.watchOptions, but there will be an options.watchOptions. In the current implementation, when the Webpack compiler watch method is called with watchOptions and the Webpack config is an array, watchOptions will be `true` and not an object.
91239ef
to
bb675fc
Compare
It worked! 💯 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thank you so much for the Pull Request. Well thorough Pull Request description and elaboration. ✅
What kind of change does this PR introduce?
bugfix
Did you add tests for your changes?
nope! I didn't see any existing tests for
processOptions
.If relevant, did you update the documentation?
I did not, but this should make the current docs on the watch options true for Webpack configs that are arrays.
Summary
webpack/webpack#4594 points out that watch options aren't being honored when the
Webpack config is an Array. In cases when it's an array, there may not
be a firstOptions.watchOptions, but there will be an options.watchOptions.
In the current implementation, when the Webpack compiler watch method is
called with watchOptions and the Webpack config is an array, watchOptions
will be
true
and not an object.I should also note that I don't have a ton of context into the intentions behind using the
watch
(which is a boolean) in the case that the other options are absent, and an object ifwatch
is false on both options and firstOptions, so at the very least my goal with this PR is to give someone who has better context the information I found so that they can make a good decision on what's best to do here.webpack/webpack#4594 (not open, but still broken)
Does this PR introduce a breaking change?
It does not, this should be a seamless fallback in the case when the Webpack config is an array instead of an object that does the right thing when the CLI has watch-* flags.
Testing/Repro
console.log
forwatchOptions
above the call tocompiler.watch
in webpack-cliwebpack-watch-poll-cfg
from the example repo to verify thewatchOptions
from a webpack.config.js works as expected and received the following:webpack-watch-poll-cli
from the example repo to verify thewatchOptions
from a CLI flag (--watch-poll
) was incorrect (with an Array config):yarn install
webpack-watch-poll-cli
from the example repo to verify thewatchOptions
from a CLI flag (--watch-poll
) works as expected with an Array config and received the following: