Don't add noscript and change onload when already exists

[alan-agius4]

When the input html contains a stylesheet which is already loaded async with the media strategy example:

<link rel="stylesheet" href="styles.40e2e5031a642e407437.css" media="print" onload="this.media='all'">
<noscript><link rel="stylesheet" href="styles.40e2e5031a642e407437.css"></noscript>

Critters will output an addition noscript tag and change the onload of the original link file.

- <link rel="stylesheet" href="styles.40e2e5031a642e407437.css" media="print" onload="this.media='all'">
+ <link rel="stylesheet" href="styles.40e2e5031a642e407437.css" media="print" onload="this.media='print'">
+ <noscript><link rel="stylesheet" href="styles.40e2e5031a642e407437.css" media="print"></noscript>
<noscript><link rel="stylesheet" href="styles.40e2e5031a642e407437.css"></noscript>

In this case while critters should extract the critical css it should not add an addition noscript tag nor change the onload to print.

[janicklas-ralph]

Can you give me some more context on when you would have lazy loaded CSS? It would seem easier to just let critters make that change

[alan-agius4]

Hi @janicklas-ralph,

Our main use-cases are typically when the same HTML document is transformed multiple times. The reasons for the multiple transformations is because the index file is used for different cases in the same application.

• App shell + SSR/Prerendering We run critters on the index.html, which contains the application shell during build time. This same document is used as the HTML document for the SSR'd page. While when doing SSR the app-shell is redundant, there are cases were users would want to selectively select pages to SSR or have a PWA togather with SSR, where the app-shell would be used when in offline mode to enhance the FCP. In this case critters will essentially be run twice on the same document, once when generating the app-shell and another time when handling the SSR request.

• CSS Async + SSR/Prerendering Similar to the above, the same HTML page is used for SSR'd and non SSR'd pages. In most cases JavaScript bundles will take a longer time to download, parse and for the application to bootstrap and start rendering the first component thus the chance of having FOUC is relatively low. Hence, we'd like to always load CSS async for client side rendered application, this will result in critters running on a document which contains link tags marked as "lazy/async".

If you are interested further about what we'd like to do you can have a look at: https://github.com/angular/angular-cli/issues/18730

For illustration purposes here's the step by step build/rendering pipeline:

(c) |- Generate browser bundle and HTML document
(o,c) |- Generate App-shell the generated browser bundle and HTML
(o,c) |- Prerender using the generated browser bundle, app-shell and HTML
(o,c) |- Start node server for SSR using the generated browser bundle, app-shell and HTML

Legend c: critters is run in this stage o: optional step

I know that these might not be the atypical use-cases that this tools was designed for.

Please let me know what you think.

[naveedahmed1]

Any update on this?

[developit]

This seems like it might be a bit tricky to detect when a given <noscript> is one Critters previously injected, versus something a developer has added. Also - @alan-agius4 I guess the standard critters inlining ends up avoiding duplication because we deduplicate selectors? Or are you setting the reduceInlineStyles:false option?

[alan-agius4]

@developit, we are using reduceInlineStyles:false.

The main reason for this is, so that between the transition from the SSR'd view to CSR view. Angular will remove/or re-use components <styles> generated on the server. However, when using reduceInlineStyles this is not possible because all styles are merged into a single style tag and causes metadata that is needed for styles to be be re-used to be lost, since this data is stored as attributed on style tags.

Overall, I am fine with closing this request. We did find a workaround to get around this.

[developit]

@alan-agius4 I think we could use this request as an interesting use-case for the plugin API @janicklas-ralph has been noodling on. It's mainly aimed at additional HTML tree reduction, but it should be possible to have it also support marking nodes as "do not mutate".