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

Development: Resolve css selector warning on prod build #8524

Closed
wants to merge 2 commits into from

Conversation

JohannesWt
Copy link
Contributor

Checklist

General

Client

Motivation and Context

When running npm run webapp:prod the following warning appeared:
image

Description

By changing the optimisation inlineCrictical to false in the angular.json file the warning is resolved. But it should be noted that this warning is a known issue in Angular 17 and should be resolved in the future. See the following GitHub (external) issues #4306 & #153

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked






Review Progress

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Test Coverage

unchanged

@JohannesWt JohannesWt added client Pull requests that update TypeScript code. (Added Automatically!) bugfix labels May 1, 2024
@JohannesWt JohannesWt self-assigned this May 1, 2024
Copy link

github-actions bot commented May 1, 2024

⚠️ Unable to deploy to test servers ⚠️

The docker build needs to run through before deploying.

@github-actions github-actions bot added the deployment-error Added by deployment workflows if an error occured label May 1, 2024
@JohannesWt JohannesWt added deploy:artemis-test3 and removed deployment-error Added by deployment workflows if an error occured labels May 1, 2024
@JohannesWt JohannesWt temporarily deployed to artemis-test3.artemis.cit.tum.de May 1, 2024 19:42 — with GitHub Actions Inactive
@Strohgelaender
Copy link
Contributor

What exactly does this option regulate? I don't think we should turn off some optimization just to silence a warning message.

@JohannesWt
Copy link
Contributor Author

What exactly does this option regulate? I don't think we should turn off some optimization just to silence a warning message.

I investigated this issue on @krusche's behalf. What the optimisation does is explained well here css-inlining
But independent of the issue there are also voices against this optimisation in general. E.g. Disable Inline critical CSS

@Strohgelaender what is your view on that?

@krusche
Copy link
Member

krusche commented May 2, 2024

Let's test how the First Contentful Paint changes when setting inlineCritical to false.
We should also investigate how much css is actually inlined and whether we can remove unsafe-inline from the SecurityConfiguration without getting any other issues. Then, we can decide better how we want to move forward with this change

@krusche
Copy link
Member

krusche commented May 2, 2024

A quick test on production shows that the index.html is 8.5kB large and takes 48ms to load from the server. It includes around 680 lines of inline CSS, mostly colors, but also a few other styles

@JohannesWt
Copy link
Contributor Author

A quick test on production shows that the index.html is 8.5kB large and takes 48ms to load from the server. It includes around 680 lines of inline CSS, mostly colors, but also a few other styles

Testing it with inline-critical=true and inline-critical=false we get the following results for FCP, DC time and total time

First Contentful Paint Document complete time Total time
Run 1 (inline=true) 3.067 s 3.340 s 3.597 s
Run 2 (inline=true) 3.101 s 3.376 s 5.197 s
Run 3 (inline=true) 3.085 s 3.463 s 3.656 s
Run 1 (inline=false) 3.068 s 3.433 s 3.433 s
Run 2 (inline=false) 3.085 s 3.395 s 3.395 s
Run 3 (inline=false) 3.135 s 3.398 s 3.398 s

We also get the following further results for HTML and CSS download size and times:

inline=true

HTML bytes 7.7 kB
HTML time 15 ms
CSS bytes 63.8 KB
CSS time 659 ms

inline=false

HTML bytes 1.7 KB
HTML time 2 ms
CSS bytes 63.8 KB
CSS time 162 ms

@github-actions github-actions bot added the server Pull requests that update Java code. (Added Automatically!) label May 2, 2024
@JohannesWt JohannesWt temporarily deployed to artemis-test3.artemis.cit.tum.de May 2, 2024 13:09 — with GitHub Actions Inactive
@JohannesWt JohannesWt closed this May 2, 2024
@JohannesWt JohannesWt deleted the bugfix/css-selector-warning-on-prod-build branch May 2, 2024 18:51
FYHenry added a commit to InoteBackup2/Inote_BrowserInterface that referenced this pull request May 28, 2024
atsuhikoMochizuki pushed a commit to InoteBackup2/Inote_BrowserInterface that referenced this pull request May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix client Pull requests that update TypeScript code. (Added Automatically!) server Pull requests that update Java code. (Added Automatically!)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants