feat: allow to configure more client options via resource URL#4274
feat: allow to configure more client options via resource URL#4274
client options via resource URL#4274Conversation
e2c6cd6 to
9dc2767
Compare
|
Let's fix test and I think we can merge it |
9dc2767 to
da958ac
Compare
|
Something wrong with tests here, looking into it. |
cd72a5c to
37db989
Compare
Codecov Report
@@ Coverage Diff @@
## master #4274 +/- ##
==========================================
+ Coverage 92.18% 92.20% +0.01%
==========================================
Files 16 16
Lines 1600 1629 +29
Branches 604 615 +11
==========================================
+ Hits 1475 1502 +27
- Misses 114 116 +2
Partials 11 11
Continue to review full report at Codecov.
|
|
|
||
| exports[`allowed hosts should connect web socket client using "[::1] host to web socket server with the "auto" value ("sockjs"): console messages 1`] = ` | ||
| Array [ | ||
| "[webpack-dev-server] Overlay is enabled for both errors and warnings.", |
There was a problem hiding this comment.
I think it is small spammy
There was a problem hiding this comment.
Hmm, we can remove this log.
There was a problem hiding this comment.
Since overlay is enabled by default this log will be printed in the console by default. And what about the progress is enabled log? Should we remove that too?
There was a problem hiding this comment.
Good question, to be honestly, it looks like very spammy currently, maybe we can join all messages in one? So developer will look one message about dev server started with HMR/overlay/etc
There was a problem hiding this comment.
Yes, [webpack-dev-server] started with HMR, live reloading, overlay, progress enabled.
f8812c6 to
c97db65
Compare
| test("should run onSocketMessage.hot", () => { | ||
| onSocketMessage.hot(); | ||
|
|
||
| expect(log.log.info.mock.calls[0][0]).toMatchSnapshot(); |
There was a problem hiding this comment.
we removed these logs from onSocketMessage.hot() because otherwise, we would have duplicate logs spamming the console.
client-src/index.js
Outdated
| }; | ||
| const parsedResourceQuery = parseURL(__resourceQuery); | ||
|
|
||
| const enabledFeatures = []; |
There was a problem hiding this comment.
In theory we will have a memory problem here, because we will put more and more, I think will:
- create general log function
- create
enabledFeaturesobject and set true/false - pass
enabledFeaturesto general log function
And it will allow to us do better logging:
[webpack-dev-server] Server started: Hot Module Replacement enabled, Live Reloading enabled, Overlay disabled.
I.e. we will output enabled and disabled, so developers can see enabled/disabled features
6d42620 to
782d815
Compare
alexander-akait
left a comment
There was a problem hiding this comment.
Looks good, but there are a lot of uncoverred lines... Maybe you can add couple tests?
|
Yeah, I'll add more tests and fix coverage separately. |
For Bugs and Features; did you add new tests?
Yes
Motivation / Use-Case
Fix #4157
Fix #4156
allow to configure
progressandoverlayoptions via resource URL.Breaking Changes
None
Additional Info
NO