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

Critters doesn't inlineing all critical css #1470

Closed
fagyikaa opened this issue Nov 8, 2020 · 6 comments
Closed

Critters doesn't inlineing all critical css #1470

fagyikaa opened this issue Nov 8, 2020 · 6 comments

Comments

@fagyikaa
Copy link

fagyikaa commented Nov 8, 2020

Do you want to request a feature or report a bug?
A bug
What is the current behaviour?
It should inline all critical css' with critter when prerendering a site but it doesn't. Critter decides if a css is critical if it's present in the DOM. However some of the css' which are in the DOM don't get inlined.
If the current behaviour is a bug, please provide the steps to reproduce.
Created a repository with a subset of my project: https://github.com/fagyikaa/preact-cli-critter-issue
Run npm build and npm serve (or just serve the already built code) then turn off your javascript to see whats get actually inlined and what doesn't. Or you can simply check out the already built html in the built directory.

What is the expected behaviour?
It should inline every css which is related to the prerendered state of DOM.
Please mention other relevant information.
Some of the css' got inlined however some of them don't. E.g. in the project preact/routes/home/hero the .HeroTitle class is not inlined while the classes in the preact/components/input do (e.g. .BaseBox). Both of them are in the prerendered html on the same page and nowhere else. A difference is that input is reexported from an index file and then used by destructuring from that index.
Maybe it's related to this issue: #908
It seems like the css' which are generated into route-app.jsx.chunk.css are inlined correctly while the others which are generated into route-home/index.jsx.chunk.css are not.

Environment Info:
System:
OS: macOS 10.15.5
CPU: (8) x64 Intel(R) Core(TM) i5-1038NG7 CPU @ 2.00GHz
Binaries:
Node: 12.18.3 - ~/.nvm/versions/node/v12.18.3/bin/node
npm: 6.14.6 - ~/.nvm/versions/node/v12.18.3/bin/npm
Browsers:
Chrome: 86.0.4240.183
Firefox: 80.0.1
Safari: 13.1.1
npmPackages:
preact: 10.3.2 => 10.3.2
preact-cli: ^3.0.0 => 3.0.0
preact-render-to-string: 5.1.4 => 5.1.4
preact-router: 3.2.1 => 3.2.1
npmGlobalPackages:
preact-cli: 3.0.1

@rschristian
Copy link
Member

Might be connected to #1393

@developit
Copy link
Member

Interesting. This seems like it points to async components like routes/* not being correctly converted to synchronous imports during prerendering...

@fagyikaa
Copy link
Author

fagyikaa commented Nov 11, 2020

Editing additionalStylesheets option of critter in config solved the issue. From ['.css'] to ['**/.css'];

if (env['inline-css'] && config.plugins[17]) { const critters = config.plugins[17]; critters.options.additionalStylesheets = ['**/*.css']; }

However i feel it's not a best practice to edit config plugins this way, if the order changes then it will break.

@rschristian
Copy link
Member

rschristian commented Nov 11, 2020

Ah, cheers. I seem to recall route resources being placed in a directory (rather than all being top level) as a recent change, though maybe I'm remembering something else.

I'll go searching tomorrow and try to get a PR together, or feel free to write one up yourself. Appreciate you investigating this.

@rschristian
Copy link
Member

Ah, in fact the issue's already been fixed with #1413 I believe.

Please upgrade Preact-CLI to latest and see if there's still an issue.

On v3.0.3 I can't reproduce.

@fagyikaa
Copy link
Author

Indeed, thank you for your help.

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

3 participants