Fix CSS ordering between imported and Astro styles (#4907)

* Fix CSS ordering between imported and Astro styles

* Fix linting errors

* Add changeset and upgrade compiler version

* Update test to reflect shared styles placed before page styles
This commit is contained in:
Matthew Phillips 2022-09-28 18:12:22 -04:00 committed by GitHub
parent 55a1b5bb58
commit 01c1aaa003
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 218 additions and 7 deletions

View file

@ -0,0 +1,34 @@
---
'astro': minor
---
Order Astro styles last, to override imported styles
This fixes CSS ordering so that imported styles are placed *higher* than page/component level styles. This means that if you do:
```astro
---
import '../styles/global.css';
---
<style>
body {
background: limegreen;
}
</style>
```
The `<style>` defined in this component will be placed *below* the imported CSS. When compiled for production this will result in something like this:
```css
/* /src/styles/global.css */
body {
background: blue;
}
/* /src/pages/index.astro */
body:where(.astro-12345) {
background: limegreen;
}
```
Given Astro's 0-specificity hashing, this change effectively makes it so that Astro styles "win" when they have the same specificity as global styles.

View file

@ -95,7 +95,7 @@
"test:e2e:match": "playwright test -g"
},
"dependencies": {
"@astrojs/compiler": "^0.24.0",
"@astrojs/compiler": "^0.25.0",
"@astrojs/language-server": "^0.26.2",
"@astrojs/markdown-remark": "^1.1.2",
"@astrojs/telemetry": "^1.0.0",

View file

@ -0,0 +1,114 @@
import { expect } from 'chai';
import * as cheerio from 'cheerio';
import { loadFixture } from './test-utils.js';
describe('CSS ordering - import order', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;
before(async () => {
fixture = await loadFixture({
root: './fixtures/css-order-import/',
});
});
/**
*
* @param {string} html
* @returns {string[]}
*/
function getLinks(html) {
let $ = cheerio.load(html);
let out = [];
$('link[rel=stylesheet]').each((i, el) => {
out.push($(el).attr('href'));
});
return out;
}
function getStyles(html) {
let $ = cheerio.load(html);
let out = [];
$('style').each((i, el) => {
out.push($(el).text());
});
return out;
}
/**
*
* @param {string} href
* @returns {Promise<{ href: string; css: string; }>}
*/
async function getLinkContent(href) {
const css = await fixture.readFile(href);
return { href, css };
}
describe('Development', () => {
/** @type {import('./test-utils').DevServer} */
let devServer;
before(async () => {
devServer = await fixture.startDevServer();
});
after(async () => {
await devServer.stop();
});
it('Page level CSS is defined lower in the page', async () => {
let res = await fixture.fetch('/');
let html = await res.text();
let [style1, style2] = getStyles(html);
expect(style1).to.include('green');
expect(style2).to.include('salmon');
});
it('import order is depth-first', async () => {
let res = await fixture.fetch('/component/');
let html = await res.text();
let [style1, style2, style3] = getStyles(html);
expect(style1).to.include('burlywood');
expect(style2).to.include('aliceblue');
expect(style3).to.include('whitesmoke');
});
});
describe('Production', () => {
before(async () => {
await fixture.build();
});
it('Page level CSS is defined lower in the page', async () => {
let html = await fixture.readFile('/index.html');
const content = await Promise.all(
getLinks(html).map((href) => getLinkContent(href))
);
const [{ css }] = content;
let idx1 = css.indexOf('salmon');
let idx2 = css.indexOf('green');
expect(idx1).to.be.greaterThan(idx2, 'Page level CSS should be placed after imported CSS');
});
it('import order is depth-first', async () => {
let html = await fixture.readFile('/component/index.html');
const content = await Promise.all(
getLinks(html).map((href) => getLinkContent(href))
);
const [{ css }] = content;
let idx1 = css.indexOf('whitesmoke');
let idx2 = css.indexOf('aliceblue');
let idx3 = css.indexOf('burlywood');
expect(idx1).to.be.greaterThan(idx2);
expect(idx2).to.be.greaterThan(idx3);
});
});
});

View file

@ -87,9 +87,10 @@ describe('CSS production ordering', () => {
);
expect(content).to.have.a.lengthOf(3, 'there are 3 stylesheets');
const [, found] = content;
const [, sharedStyles, pageStyles] = content;
expect(found.css).to.match(/#00f/);
expect(sharedStyles.css).to.match(/red/);
expect(pageStyles.css).to.match(/#00f/);
});
it('CSS injected by injectScript comes first because of import order', async () => {

View file

@ -0,0 +1,6 @@
{
"name": "@test/css-order-import",
"dependencies": {
"astro": "workspace:*"
}
}

View file

@ -0,0 +1,4 @@
---
import '../styles/One.css';
---
<link>

View file

@ -0,0 +1,5 @@
<style>
body {
background: aliceblue;
}
</style>

View file

@ -0,0 +1,19 @@
---
import One from '../components/One.astro';
import Two from '../components/Two.astro';
---
<html>
<head>
<title>Test</title>
<One />
<Two />
<style>
body {
background: whitesmoke;
}
</style>
</head>
<body>
<h1>Test</h1>
</body>
</html>

View file

@ -0,0 +1,16 @@
---
import '../styles/base.css';
---
<html>
<head>
<title>Test</title>
<style>
body {
background: salmon;
}
</style>
</head>
<body>
<h1>Test</h1>
</body>
</html>

View file

@ -0,0 +1,3 @@
body {
background: burlywood;
}

View file

@ -0,0 +1,3 @@
body {
background: green;
}

View file

@ -349,7 +349,7 @@ importers:
packages/astro:
specifiers:
'@astrojs/compiler': ^0.24.0
'@astrojs/compiler': ^0.25.0
'@astrojs/language-server': ^0.26.2
'@astrojs/markdown-remark': ^1.1.2
'@astrojs/telemetry': ^1.0.0
@ -443,7 +443,7 @@ importers:
yargs-parser: ^21.0.1
zod: ^3.17.3
dependencies:
'@astrojs/compiler': 0.24.0
'@astrojs/compiler': 0.25.0
'@astrojs/language-server': 0.26.2
'@astrojs/markdown-remark': link:../markdown/remark
'@astrojs/telemetry': link:../telemetry
@ -1584,6 +1584,12 @@ importers:
dependencies:
astro: link:../../..
packages/astro/test/fixtures/css-order-import:
specifiers:
astro: workspace:*
dependencies:
astro: link:../../..
packages/astro/test/fixtures/custom-404:
specifiers:
astro: workspace:*
@ -3639,8 +3645,8 @@ packages:
resolution: {integrity: sha512-vBMPy9ok4iLapSyCCT1qsZ9dK7LkVFl9mObtLEmWiec9myGHS9h2kQY2xzPeFNJiWXUf9O6tSyQpQTy5As/p3g==}
dev: false
/@astrojs/compiler/0.24.0:
resolution: {integrity: sha512-xZ81C/oMfExdF18I1Tyd2BKKzBqO+qYYctSy4iCwH4UWSo/4Y8A8MAzV1hG67uuE7hFRourSl6H5KUbhyChv/A==}
/@astrojs/compiler/0.25.0:
resolution: {integrity: sha512-thJdIFuKT7f6uzxUs5d7qjh3g/L4kmlSfddAcbC62QEMy4H9N9fig3+FBwg9exUvXYcqOjZ/nC2PsGsVIqmZYA==}
dev: false
/@astrojs/language-server/0.26.2: