fix: overlay with warnings

[alexander-akait]

• This is a bugfix
• This is a feature
• This is a code refactor
• This is a test update
• This is a docs update
• This is a metadata update

For Bugs and Features; did you add new tests?

bug, existing

Motivation / Use-Case

When we set overlay: true we mean overlay: { warnings: true, errors: true }, but current logic is not show warnings in overlay by defualt, also we have bug when we show warning and invalidate code warnings just hide

Breaking Changes

In theory no, because overlay: true should show warnings and errors

Additional Info

No

[alexander-akait]

Here we fix bug, when if we have previously warning and run invalidate without change code (for example using API to invalidate), overlay with warnings hide and did not show again

[alexander-akait]

Better keep options here as is,in future we can improve this and allow to using this http://192.168.0.5?overlay&no-progress

[snitin315]

Let's update snapshot

[jordanbtucker]

This is a breaking change, by the way. It doesn't really matter what {overlay: true} should have done. It has always meant {overlay: {errors: true}} according to the documentation. If you're making a change to the public API, then it's a breaking change. So, if this is merged, it should be called out as a BREAKING CHANGE in the release notes.

[alexander-akait]

@jordanbtucker no problems, docs will be updated together with stable release

[codecov]

Codecov Report

Merging #3215 (16fb784) into master (d2ef7e8) will increase coverage by 0.11%.
The diff coverage is 95.83%.

@@            Coverage Diff             @@
##           master    #3215      +/-   ##
==========================================
+ Coverage   95.18%   95.29%   +0.11%     
==========================================
  Files          37       34       -3     
  Lines        1247     1234      -13     
  Branches      351      350       -1     
==========================================
- Hits         1187     1176      -11     
+ Misses         54       52       -2     
  Partials        6        6              

Impacted Files	Coverage Δ
client-src/overlay.js	97.01% <ø> (ø)
client-src/index.js	90.56% <95.00%> (-0.87%)	⬇️
client-src/utils/sendMessage.js	85.71% <100.00%> (ø)
lib/Server.js	95.16% <100.00%> (+0.61%)	⬆️
lib/utils/findPort.js	100.00% <0.00%> (ø)
lib/utils/defaultPort.js
lib/utils/updateCompiler.js
lib/utils/tryParseInt.js

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d2ef7e8...16fb784. Read the comment docs.

[snitin315]

LGTM

[Birch-san]

to get the old behaviour (show only errors in overlay): am I understanding correctly that the new convention is supposed to be:

overlay: {
  warnings: false,
  errors: true
}

from this PR, it looks like this object-style syntax is supposed to be supported. but it's rejected in schema validation:

configuration has an unknown property 'overlay'. These properties are valid:
object { bonjour?, client?, compress?, …

I've also tried overlay: false, but this isn't supported either.

As a result, my application is covered with a full-screen overlay warning me of my bundle size. this is not something I can trivially fix — I would rather hide the overlay, but don't know how.

I'm using 4.0.0-beta.3.

[Birch-san]

ah, I see; it's:

client: {
  overlay:  {
    warnings: false,
    errors: true
  }
}

[alexander-akait]

@Birch-san Yes, we will update docs in future, anyway why you disable warnings? What is the problem?

[Birch-san]

@alexander-akait
disabled warnings because it's a warning about "chunk size is bigger than recommended". we know that 50MB is a big chunk, but we don't really have the option of making it smaller — we're limiting our app to 6 chunks to fit into browser's HTTP 1.1 concurrent request limit.

correct solution is to serve via HTTP/2 (so we can have as many chunks as we like), but requires us to get SSL certificates, so there's a few things we'd need to get right. we've accepted for now that it's easier to keep the big chunks.

out of interest: how bad is it to have big chunks? what kind of problems happen?

[alexander-akait]

You can disable/change/filter this warning using https://webpack.js.org/configuration/performance/#performancehints

out of interest: how bad is it to have big chunks? what kind of problems happen?

Big chunk is always not good, but splitting them is not simple story for some projects, need look at code

[Birch-san]

disable/change/filter this warning

thanks; I'm still happy to see the warning in the webpack CLI. only wanted it excluded from the full-screen overlay.

splitting them is not simple story for some projects, need look at code

can't show code. will describe broad approach…

we currently chunk based on how long they're likely to live in the browser's cache (or whether they're conditionally-used features):

• libraries that change rarely
• libraries that change semi-frequently
• dynamically-imported libraries
• everything else

I think those categories are good (are they? 🤔 ), but if we used more chunks… probably I'd use the same categories, but split each category into 250kB chunks?

it looks like maybe it wouldn't be impossible for us to enable HTTP/2 and use small chunks… are there any problems that can happen when you have loads of small chunks? or is this the recommended way to do chunking?

[alexander-akait]

are there any problems that can happen when you have loads of small chunks? or is this the recommended way to do chunking?

With http2 I will say it is recommend to split on small chunks, without http2 just don't create a l lot requests in the same time (most of browsers support only 5/10 parallel requests with http1.1)

You can lazy load part of your code when it will be used, for example you don't need logic for checkout while user on other page

[Birch-san]

thanks — then, might be worth an investigation. would be nice to get down to zero warnings! 🙂