-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
feat: add watchFiles option #3136
Conversation
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.
Also we need better test, look on test in test/server/liveReload-option.test.js
Codecov Report
@@ Coverage Diff @@
## master #3136 +/- ##
==========================================
+ Coverage 94.94% 94.99% +0.04%
==========================================
Files 37 37
Lines 1206 1218 +12
Branches 332 338 +6
==========================================
+ Hits 1145 1157 +12
Misses 55 55
Partials 6 6
Continue to review full report at Codecov.
|
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.
Also we need CLI option --watch-files /foo/
(can be multiple)
I will finish this PR, need small improvements |
26159ca
to
681f8d7
Compare
/cc @anshumanv @snitin315 I make occasional mistakes sometimes, it would be nice to get more eyes |
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, thanks!
lib/options.json
Outdated
"port": "should be {Number|String|Null} (https://webpack.js.org/configuration/dev-server/#devserverport)", | ||
"proxy": "should be {Object|Array} (https://webpack.js.org/configuration/dev-server/#devserverproxy)", | ||
"public": "should be {String} (https://webpack.js.org/configuration/dev-server/#devserverpublic)", | ||
"setupExitSignals": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserversetupexitsignals)", | ||
"static": "should be {Boolean|String|Object|Array} (https://webpack.js.org/configuration/dev-server/#devserverstatic)", | ||
"transportMode": "should be {String|Object} (https://webpack.js.org/configuration/dev-server/#devservertransportmode)" | ||
"transportMode": "should be {String|Object} (https://webpack.js.org/configuration/dev-server/#devservertransportmode)", | ||
"watchFiles": "should be {String|Array} (https://webpack.js.org/configuration/dev-server/#devserverwatchfiles)" |
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.
it can also be object
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.
Just not, this is not work right now, we need move them to description
property, also remove types, our default validator already print them in pretty way, just test
Should be easy task
will add docs after release |
bin/cli-flags.js
Outdated
@@ -228,5 +228,16 @@ module.exports = { | |||
multiple: true, | |||
negative: true, | |||
}, | |||
{ | |||
name: 'watch-files', | |||
type: [String], |
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.
type: [String], | |
type: String, |
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
For Bugs and Features; did you add new tests?
wip on tests
Motivation / Use-Case
Add watchFiles option to listen to user-defined file changes
Breaking Changes
Nope
Additional Info
Fix #3121