Skip to content

Commit

Permalink
fix: assume non-class name selectors to be critical (#250)
Browse files Browse the repository at this point in the history
For apps using a plugin such as MiniCssExtractPlugin and importing plain CSS files, the global CSS will often be in the same CSS file as the CSS extracted by Linaria. Previously, collect would place these styles in 'other', which would cause a FOUC. There's an undocumented 'globalCSS' argument, but it's not reasonable to manually specify it when you have already imported it in the app.

This PR only checks class name based selectors to determine if they are critical and for all other styles, it will place them in the critical CSS. This handles global CSS without any manual work. This also removes the 'globalCSS' argument because it's not needed anymore.
  • Loading branch information
satya164 authored Oct 14, 2018
1 parent 08af9aa commit 19192ac
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 22 deletions.
6 changes: 5 additions & 1 deletion docs/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ const FancyButton = styled(Button)`

### `collect(html: string, css: string) => string`

Takes HTML and CSS strings and returns the critical CSS used in the page by analyzing the class names. It can be used to detrmine critical CSS for server side rendering.
Takes HTML and CSS strings and returns the critical CSS used in the page by analyzing the class names. It can be used to determine critical CSS for server side rendering.

```js
import { collect } from 'linaria/server';
Expand All @@ -146,3 +146,7 @@ const { critical, other } = collect(html, css);
// critical – returns critical CSS for given html
// other – returns the rest of styles
```

This will only detect critical CSS based on class names, so if you have any other type of selectors, they'll get added to the critical CSS.

Also note that extracting critical CSS this way will change the order of class names. It's not a problem if you're primarily using Linaria for styling. However if you're using a third party framework which imports its own CSS, then it's not recommended to use this helper on the extracted CSS.
2 changes: 0 additions & 2 deletions src/server/__tests__/__snapshots__/collect.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -274,11 +274,9 @@ exports[`works with global css critical 1`] = `
"body {
font-size: 13.37px;
}
html {
-webkit-font-smoothing: antialiased;
}
h1 {
font-weight: bold;
}
Expand Down
21 changes: 7 additions & 14 deletions src/server/__tests__/collect.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,28 +183,21 @@ describe('works with pseudo-class and pseudo-elements', () => {

describe('works with global css', () => {
const css = dedent`
body { font-size: 13.37px; }
html { -webkit-font-smoothing: antialiased; }
h1 { font-weight: bold; }
.linaria:active {}
.linaria::before {}
.other:active {}
.other::before {}
`;

const globalCSS = dedent`
body {
font-size: 13.37px;
}
html {
-webkit-font-smoothing: antialiased;
}
h1 {
font-weight: bold;
}
`;
const { critical, other } = collect(html, css);

const { critical, other } = collect(html, css, globalCSS);
test('critical', () => expect(prettyPrint(critical)).toMatchSnapshot());
test('other', () => expect(prettyPrint(other)).toMatchSnapshot());
});
18 changes: 13 additions & 5 deletions src/server/collect.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,28 @@ type CollectResult = {

const collect = (
html /* : string */,
css /* : string */,
globalCSS /* : ?string */
css /* : string */
) /* : CollectResult */ => {
const animations = new Set();
const other = postcss.root();
const critical = postcss.root();
const stylesheet = postcss.parse(css);
const htmlClassesRegExp = extractClassesFromHtml(html);

const isCritical = rule => {
// Only check class names selectors
if (rule.selector.startsWith('.')) {
return Boolean(rule.selector.match(htmlClassesRegExp));
}

return true;
};

const handleAtRule = rule => {
let addedToCritical = false;

rule.each(childRule => {
if (childRule.selector.match(htmlClassesRegExp)) {
if (isCritical(childRule)) {
critical.append(rule.clone());
addedToCritical = true;
}
Expand All @@ -51,7 +59,7 @@ const collect = (
return;
}

if (rule.selector.match(htmlClassesRegExp)) {
if (isCritical(rule)) {
critical.append(rule);
} else {
other.append(rule);
Expand All @@ -69,7 +77,7 @@ const collect = (
});

return {
critical: (globalCSS || '') + critical.toString(),
critical: critical.toString(),
other: other.toString(),
};
};
Expand Down

0 comments on commit 19192ac

Please sign in to comment.