angular / angular-cli

CLI tool for Angular
https://cli.angular.dev
MIT License
26.79k stars 11.98k forks source link

optimization/minify in production makes Angular Material styles not load consistently #23384

Closed davidtlee closed 2 years ago

davidtlee commented 2 years ago

🐞 Bug report

Command (mark with an x)

Is this a regression?

Yes, the previous version in which this bug was not present was: Angular 10.

Description

  1. In Angular 13, when I click a dropdown for the Angular Material MatMenu, it will sometimes position the dropdown options underneath the bottom of the page. I can get it to do it reliably by clicking "Refresh" in my browser over and over (it happens roughly 50% of the time so I can usually get the behavior to come out with the 4th refresh or so).

  2. When I inspect the css in DevTools, I see that cdk-overlay-container does not have the styles like position: fixed; that place it in the right location. In other words, the material styles do not seem to have loaded.

  3. This bug does not occur in development, only when building (ng build) or serving in production (ng serve --configuration production). Moreover, if I set optimization: false within angular.json for production, the bug also disappears. This is why I am posting this issue within the angular-cli repo as I am guessing that the issue stems from the minification process or something else in the optimization process.

  4. In looking around, a lot of other people who have experienced the same Angular Material issue experience it when they do not load the prebuilt themes (e.g. https://github.com/angular/components/issues/1387), but I do have the prebuilt themes loaded correctly (which is why for me it works around 50% of the time, but doesn't work 50% of the time). I was able to find one other case in which others experienced "random" occurrences of missing styles like what I'm experiencing. In that issue (links below), it was found that "Chrome loads styles for Material later then application starts rendering in Angular". So my guess is that the bug I'm experiencing is also related to the timing of when styles are being loaded.

Any chance all of those clues mean something to someone?

πŸ”¬ Minimal Reproduction

I haven't quite been able to create a simple reproduction in Stackblitz, but I got pretty close with this Stackblitz: https://stackblitz.com/edit/components-issue-afy6ka?file=src%2Fapp%2Fexample-component.html,src%2Fapp%2Fexample-component.css,src%2Fstyles.scss,angular.json. When I first created that stackblitz and started working towards a reproduction, I recreated the problem by just removing the aot: true from angular.json and adding a simple example of a mat-menu within a page with height: 100%; for html, body, and all containers. However, once I used the "Share" link from the Stackblitz, the problem wasn't there anymore. I created a screen recording to show that I really did create the effect in this Stackblitz, but it somehow disappeared when I Shared it and also after I refreshing the browser

https://user-images.githubusercontent.com/863081/174004841-7edb886f-f8dc-4085-8bc5-3dea3fc883c2.mov

I'm hoping that the combination of all these clues will give someone an idea of what the issue is.

πŸ”₯ Exception or Error

No exceptions or errors.

🌍 Your Environment


Angular CLI: 13.2.5
Node: 16.10.0
Package Manager: npm 7.24.2
OS: darwin x64

Angular: 13.2.5
... animations, cdk, cli, common, compiler, compiler-cli, core
... forms, language-service, localize, material
... platform-browser, platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1302.5
@angular-devkit/build-angular   13.2.5
@angular-devkit/core            13.2.5
@angular-devkit/schematics      13.2.5
@angular/fire                   7.2.1
@schematics/angular             13.2.5
ng-packagr                      13.2.1
rxjs                            7.4.0
typescript                      4.5.5
webpack                         5.70.0

Anything else relevant?

I wrote all the clues I've observed in the "Description" section.

alan-agius4 commented 2 years ago

While, I cannot say for certain what is the problem as I am unable to replicate this.

One thing I noticed is that the index.html is malformed and isn't a valid HTML document as it has several missing elements. Can you try and see if you are able to replicate this when fixing your index to the below;

<!doctype html>
<html lang="en">
<head>
  <meta charset="utf-8">
  <base href="/">

  <meta name="viewport" content="width=device-width, initial-scale=1">
</head>
<body>
     <link href="https://fonts.googleapis.com/icon?family=Material+Icons&display=block" rel="stylesheet">
     <div class="mat-app-background basic-container">
       <example-component>loading</example-component>
     </div>
     <span class="version-info">Current build: 9.0.1</span>
</body>
</html>
davidtlee commented 2 years ago

The index.html file is unchanged from this template provided for submitting bug issues in the angular/components repo: https://stackblitz.com/fork/components-issue. In my actual repo where I'm experiencing this, the index.html is normal. But I am realizing that my attempt to reproduce in stackblitz that I captured in the screen recording probably is actually coming from the fact that I commented out the prebuilt-themes import in styles.scss and instead imported it in angular.json styles array. So that's why I saw the "error" appear until I refreshed but then couldn't get it to come back afterwards. This is not the same error as the one I'm experiencing in which the error only occurs 50% of the time.

I've made my actual repo public and created a branch that reproduces the bug here: https://github.com/tech4good-lab/tech4good.io/tree/replicate-bug (although it isn't not a "minimal" reproduction, hopefully it's helpful). Note that by changing optimization: true to optimization: false, in angular.json, you can reliably shut down the bug.

alan-agius4 commented 2 years ago

The root cause appears to be that in version 12, we enabled critical css by default. One of the effects of this is that stylesheets are updated to be non render blocking. In your particular case however in style.scss you are requesting an external resource which causes the stylesheet to finish download post bootstrapping. The stylesheet in question also takes a considerable amount of time to download. 2.5s on my machine.

@import url(//db.onlinewebfonts.com/c/47ff3156fe928e750d0468143555356f?family=SofiaProSoftW01-Regular);

You have a number of options to address this issue. 1) Copy the CSS to your stylesheet 2) Find an alternative provider that provides this font with a reasonable download time. 4) Reference to this resource in the index.html page. 3) Disable critical css inlining. To disable critical css you can update the optimization option to the below:

"optimization": {
   "scripts": true,
   "styles": {
      "minify": true,
      "inlineCritical": false
    }
 }
davidtlee commented 2 years ago

Thank you! It does seem like disabling critical css inlining solves the problem.

I also tried to keep critical css inlining by completely getting rid of Sofia Pro and replacing it with Nunito Sans (so that I no longer have an external file request in my styles.scss), but this does not seem to have worked. I have pushed the commit that gets rid of that external resource request and for which the bug still seems to be occurring. Do you have any ideas what's the problem now?

alan-agius4 commented 2 years ago

Hi @davidtlee,

I had another look at this and looking at the generated HTML at runtime, I also noticed that the link element is being changed to an inline style with a media print value.

<link rel="stylesheet" href="styles.css" media="print" onload="this.media='all'">
<style media="print" data-href="styles.css">
...
</style>

This seems to be caused by one of the scripts you are using https://cdnjs.cloudflare.com/ajax/libs/prefixfree/1.0.7/prefixfree.min.js

The main goal of the above script appears to be adding vendor prefixes. This makes this script redundant as Angular CLI adds the necessary vendor prefixes at build time.

Closing as this is not a caused by Angular.

davidtlee commented 2 years ago

@alan-agius4 that seems to have solved it! thank you for all your help!

angular-automatic-lock-bot[bot] commented 2 years ago

This issue has been automatically locked due to inactivity. Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.