fix: add client.overlay.runtimeErrors options #4773
Conversation
malcolm-kee
requested review from
alexander-akait,
hiroppy,
snitin315 and
anshumanv
as code owners
|
@alexander-akait I added the fix and verified it by testing it manually. If automated test is a must-have I'll have to do it tomorrow as I'm very tired now (it's 3am here). |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #4773 +/- ##
==========================================
- Coverage 92.10% 90.43% -1.68%
==========================================
Files 16 16
Lines 1659 1662 +3
Branches 618 622 +4
==========================================
- Hits 1528 1503 -25
- Misses 120 145 +25
- Partials 11 14 +3
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
client-src/index.js
Outdated
| const overlay = createOverlay({ | ||
| trustedTypesPolicyName, | ||
| }); | ||
| const overlay = options.overlay |
There was a problem hiding this comment.
@malcolm-kee this is not sufficient. You are only checking if options.overlay is truthy. All options need to be passed into the createOverlay() function. The overlay options are errors: boolean & warnings: boolean.
Then, the show() function in overlay.js needs to check if the type is "warning" or "error" and compare the passed options.
There was a problem hiding this comment.
Yeah, we have before https://github.com/webpack/webpack-dev-server/blob/v4.11.1/client-src/index.js#L253
There was a problem hiding this comment.
@crobinson42 good catch!
I just pushed a fix so that runtime error check the options as well.
For build error/warning, it's already handled:
There was a problem hiding this comment.
Testa are failed, can you look what is wrong (no rush), I think today after rest we will fix all issues
client-src/index.js
Outdated
| typeof options.overlay === "object" | ||
| ? { | ||
| trustedTypesPolicyName: options.overlay.trustedTypesPolicyName, | ||
| catchRuntimeError: options.overlay.errors, |
There was a problem hiding this comment.
Also ideally we need a new option for such stuff, i.e. catchRuntimeError, because as you can see some application works in very differents envs and it can be errors outside own application
| @@ -80,6 +80,7 @@ if (parsedResourceQuery.overlay) { | |||
| options.overlay = { | |||
| errors: true, | |||
| warnings: true, | |||
| runtimeErrors: true, | |||
There was a problem hiding this comment.
Default it to true.
malcolm-kee
changed the title
|
@alexander-akait I'm having problem to run the test for webpack5 to update snapshot. I tried to run it it will stuck without any reason. It's the same with |
|
I will update tests don't worry |
Reported in #4771
fixes #4774
fixes #4771
client.overlay.runtimeErrorsoptions - default totrue.For Bugs and Features; did you add new tests?
Motivation / Use-Case
Breaking Changes
Additional Info