bezoerb / inline-critical

Inline critical path CSS and async load existing stylesheets
Other
115 stars 15 forks source link

Preloading strategy & uncritical css #292

Closed robisim74 closed 2 years ago

robisim74 commented 2 years ago

Hi @bezoerb,

I too have to face the CSP problem, like: https://github.com/addyosmani/critical/issues/480 so now I can't use the mediastrategy (and also swap & polyfill).

But using the other strategies I get in my html:

Using default strategy with extract=true

<head>
    <style>
        /* inlined  */
    </style>
    <link href="css/index.518624788d8bb4e2cd8b.015276a2.css" rel="preload" as="style"> /* uncritical */
</head>

<body>
    <link rel="stylesheet" href="./css/index.518624788d8bb4e2cd8b.css">  /* original css */
</body>

Problems: the css classes are duplicated, because the original stylesheet contains inlined & uncritical classes.

and generates in Chrome the message: The resource http://localhost:8080/css/index.518624788d8bb4e2cd8b.015276a2.css was preloaded using link preload but not used within a few seconds from the window's load event. Please make sure it has an appropriate as value and it is preloaded intentionally.

Using body strategy with extract=true

<head>
    <style>
        /* inlined  */
    </style>
    /* missing uncritical */
</head>

<body>
    <link rel="stylesheet" href="./css/index.518624788d8bb4e2cd8b.css">  /* original css */
</body>

Problems: uncritical file is missing, and inlined css classes are duplicated, because the original stylesheet contains inlined classes.

Expected behavior Even if asynchronous loading of css doesn't work with CSP, I would at least get the inlining and loading of uncritical css, for example:

<head>
    <style>
        /* inlined  */
    </style>
    <link rel="stylesheet" href="css/index.518624788d8bb4e2cd8b.015276a2.css"> /* uncritical */
</head>

<body>
 /* no original css */
</body>

But at the moment this does not seem possible, and I have to give up using extract=true.

Thanks

bezoerb commented 2 years ago

Hey @robisim74, thanks for this detailed bug report :) I'm currently very busy but hopefully i will find some spare time to look into this after Christmas. Would it be possible for you to create a gist or a simple demo setup to reproduce the issue? This would make debugging a lot easier :)

robisim74 commented 2 years ago

Yes, you can find a starter repo here: https://github.com/robisim74/nodejs-mvc

Just add extract: true to the options of inline-critical in build.js file: https://github.com/robisim74/nodejs-mvc/blob/main/build.js#L48-L50

Then run npm run build : you'll find the result in dist\views\index.html

To see what happens in the browser, run npm start

Hope this helps!

If you agree, I can also try to do a pr.

bezoerb commented 2 years ago

Hey @robisim74, this should be fixed in v9.0.1

Thanks for the starter repo :)

Would you mind trying if everything works as expected?

robisim74 commented 2 years ago

Thanks! Tested, it works well.

Greetings