Skip to content
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

Avoid render-blocking requests #949

Open
evantd opened this issue Jun 3, 2022 · 8 comments
Open

Avoid render-blocking requests #949

evantd opened this issue Jun 3, 2022 · 8 comments

Comments

@evantd
Copy link

evantd commented Jun 3, 2022

Feature Proposal

Either change the default or add an option for making CSS bundle requests non-render-blocking. This is currently possible by using a custom insert function, but an explicit option or even changing the default would be nice.

This feels like it could be risky, but I'm having trouble coming up with any concrete concerns.

Feature Use Case

Dynamically imported modules with associated CSS currently trigger warnings about render-blocking requests in Chrome's Performance Insights tool. Since webpack uses Promises to block execution of the module until the CSS chunks are loaded, the CSS chunks don't need to block any other rendering on the page.

You can see those render-blocking requests here (after DOM Content Loaded, since that's when we do the dynamic imports):

Screen Shot 2022-06-02 at 8 22 28 PM

We can prevent the requests from blocking general rendering by setting media='print' before inserting the link tag, and then switching it to media='all' once it loads. This is currently possible by using a custom insert function, like:

                      // Don't block rendering during the network fetch. Insert link tag with media=print and then only swap it to media=all once it has loaded.
                      // eslint-disable-next-line
                      insert: function (linkTag) {
                          // eslint-disable-next-line
                          var oldOnLoad = linkTag.onload;
                          linkTag.onload = function (event) {
                              linkTag.media = 'all';
                              oldOnLoad(event);
                          };
                          linkTag.media = 'print';
                          document.head.appendChild(linkTag);
                      },

Here you can see that doing so removes the render-blocking request warnings from the Performance Insights (from after DCL):

Screen Shot 2022-06-02 at 8 22 41 PM

Please paste the results of npx webpack-cli info here, and mention other relevant information

% npx webpack-cli info

  System:
    OS: Linux 5.13 Ubuntu 20.04.4 LTS (Focal Fossa)
    CPU: (8) x64 Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
    Memory: 5.32 GB / 30.90 GB
  Binaries:
    Node: 14.19.3 - ~/.volta/tools/image/node/14.19.3/bin/node
    npm: 6.14.17 - ~/.volta/tools/image/npm/6.14.17/bin/npm
  Packages:
    html-webpack-plugin: ^5.5.0 => 5.5.0
    webpack-manifest-plugin: ^5.0.0 => 5.0.0
    webpack-node-externals: ^1.7.2 => 1.7.2
    webpack-retry-chunk-load-plugin: ^2.2.0 => 2.2.0
    webpack-stats-plugin: ^1.0.3 => 1.0.3

mini-css-extract-plugin in my project is at 1.6.2, which I realize is old, but this appears to still be relevant to the code on master.

@alexander-akait
Copy link
Member

I can accept it under option, otherwise you can flash of styles

@evantd
Copy link
Author

evantd commented Jun 3, 2022

Can you say more about how this could lead to a flash? Since the module won't be executed until after the CSS chunk's Promise resolves (and it only resolves after setting media='all'), I was thinking that a flash shouldn't be any more possible than the current way of doing things.

@alexander-akait
Copy link
Member

Because this code executed only for async CSS, you can faced with a situation, when your style was loaded (but not rendered, so you got broken print styles) and you press print (it is exotic situation, but it is possible). And verse vice.

Also you can get a sutiation when you load multiple async css, so which loaded firstly (and mostly with small size) will rendered firstly and then other, it can have negative expientcy with your interfaces (different flash and layout shifting)

Also there is situation when you can use not all for link (so we should provide option/callback to setup it).

We use blocking loading by default to prevent any racing, but yes if you follow certain rules, you can get rid of these situations

@evantd
Copy link
Author

evantd commented Jun 6, 2022

If I'm correctly understanding the use-cases you described, I think the dynamically loaded CSS would have to affect existent content (rather than content that will be added by the JavaScript part of the chunk). In that case, though, I think the same flashes would occur without my change.

Or maybe the case you're talking about is one where the chunk with the CSS is requested, and then the code that requested the chunk starts rendering content that the CSS will apply to without waiting on the Promise?

@alexander-akait
Copy link
Member

I mean if you can have multiple async CSS chunks they will be rendered in race, it can have bad experience with your UI, If there were no effects, then browsers would default to non-blocking mode.

@kelvinatanael
Copy link

Any resolution for this problem? Experiencing the same problem to avoid the blocking css.

Suggestion: Why not make the option to change or not change the onload function through the insert method optional and developers define whether or not they can change this behavior on their website?

Maybe put a property that can handle the blocking css within the plugin's webpack onload code.

@rcherny
Copy link

rcherny commented Nov 23, 2023

I think some of the comments are missing a fairly common use case here. In our case, for example, we have a very large code base (still working on optimizing this) and we will have loaded the critical rendering CSS already. When we swap out a route in a SPA or load and execute an “island” in the page we don’t want to block the rendering or execution thread. We also routinely load multiple modules as the page is loading. The outside framework will have loaded and rendered already. We want the “work” to continue without being blocked.

Perhaps the team is looking to avoid a potential “footgun” but this could be opt-in for a given set of files. So in our case this is a large code base performance issue. Often times I feel like there’s an assumption people are trying to load everything this way but that’s not the case. Let the developer decide.

@alexander-akait
Copy link
Member

I am fine with an option, but we need more dicussion how to implement this with real use case, so I am awating some feedback here, sorry for delay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants