Skip to content

Commit

Permalink
Improve CSS import ordering, fix empty CSS outputs (#2081)
Browse files Browse the repository at this point in the history
Fixes #2060
  • Loading branch information
drwpow committed Dec 2, 2021
1 parent f6b15c3 commit 62a5e98
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 12 deletions.
5 changes: 5 additions & 0 deletions .changeset/clever-suits-hide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Bugfix: CSS import ordering, empty CSS output on build
2 changes: 2 additions & 0 deletions packages/astro/src/vite-plugin-build-css/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin {
}
}

if (!chunkCSS) return null; // don’t output empty .css files

if (isPureCSS) {
const { code: minifiedCSS } = await esbuild.transform(chunkCSS, {
loader: 'css',
Expand Down
48 changes: 40 additions & 8 deletions packages/astro/src/vite-plugin-build-html/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export function rollupPluginAstroBuildHTML(options: PluginOptions): VitePlugin {
const astroAssetMap = new Map<string, Promise<Buffer>>();

const cssChunkMap = new Map<string, string[]>();
const pageStyleImportOrder: string[] = [];

return {
name: PLUGIN_NAME,
Expand Down Expand Up @@ -176,6 +177,11 @@ export function rollupPluginAstroBuildHTML(options: PluginOptions): VitePlugin {
const jsSource = assetImports.map((sid) => `import '${sid}';`).join('\n');
astroPageStyleMap.set(pageStyleId, jsSource);
assetInput.add(pageStyleId);

// preserve asset order in the order we encounter them
for (const assetHref of assetImports) {
if (!pageStyleImportOrder.includes(assetHref)) pageStyleImportOrder.push(assetHref);
}
}
}
}
Expand Down Expand Up @@ -260,17 +266,43 @@ export function rollupPluginAstroBuildHTML(options: PluginOptions): VitePlugin {
assetIdMap.set(assetPath, referenceId);
}

// Sort CSS in order of appearance in HTML (pageStyleImportOrder)
// This is the “global ordering” used below
const sortedCSSChunks = [...pureCSSChunks];
sortedCSSChunks.sort((a, b) => {
let aIndex = Math.min(
...Object.keys(a.modules).map((id) => {
const i = pageStyleImportOrder.findIndex((url) => id.endsWith(url));
return i >= 0 ? i : Infinity; // if -1 is encountered (unknown order), move to the end (Infinity)
})
);
let bIndex = Math.min(
...Object.keys(b.modules).map((id) => {
const i = pageStyleImportOrder.findIndex((url) => id.endsWith(url));
return i >= 0 ? i : Infinity;
})
);
return aIndex - bIndex;
});
const sortedChunkNames = sortedCSSChunks.map(({ fileName }) => fileName);

// Create a mapping of chunks to dependent chunks, used to add the proper
// link tags for CSS.
for (const chunk of pureCSSChunks) {
const chunkReferenceIds: string[] = [];
for (const [specifier, chunkRefID] of chunkToReferenceIdMap.entries()) {
if (chunk.imports.includes(specifier) || specifier === chunk.fileName) {
chunkReferenceIds.push(chunkRefID);
}
for (const chunk of sortedCSSChunks) {
const chunkModules = [chunk.fileName, ...chunk.imports];
// For each HTML output, sort CSS in HTML order Note: here we actually
// want -1 to be first. Since the last CSS “wins”, we want to load
// “unknown” (-1) CSS ordering first, followed by “known” ordering at
// the end so it takes priority
chunkModules.sort((a, b) => sortedChunkNames.indexOf(a) - sortedChunkNames.indexOf(b));

const referenceIDs: string[] = [];
for (const chunkID of chunkModules) {
const referenceID = chunkToReferenceIdMap.get(chunkID);
if (referenceID) referenceIDs.push(referenceID);
}
for (const [id] of Object.entries(chunk.modules)) {
cssChunkMap.set(id, chunkReferenceIds);
for (const id of Object.keys(chunk.modules)) {
cssChunkMap.set(id, referenceIDs);
}
}

Expand Down
7 changes: 3 additions & 4 deletions packages/astro/test/astro-css-bundling-import.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('CSS Bundling (ESM import)', () => {
await fixture.build();
});

it.skip('CSS output in import order', async () => {
it('CSS output in import order', async () => {
// note: this test is a little confusing, but the main idea is that
// page-2.astro contains all of page-1.astro, plus some unique styles.
// we only test page-2 to ensure the proper order is observed.
Expand All @@ -29,11 +29,10 @@ describe('CSS Bundling (ESM import)', () => {
expect(css.indexOf('p{color:green}')).to.be.greaterThan(css.indexOf('p{color:red}'));

// test 2: insure green comes after blue (page-1.css)
expect(css.indexOf('p{color:green}')).to.be.greaterThan(css.indexOf('p{color:red}'));
expect(css.indexOf('p{color:green}')).to.be.greaterThan(css.indexOf('p{color:#00f}'));
});

// TODO: need more investigation to fix this
it.skip('no empty CSS files', async () => {
it('no empty CSS files', async () => {
for (const page of ['/page-1/index.html', '/page-2/index.html']) {
const html = await fixture.readFile(page);
const $ = cheerio.load(html);
Expand Down

0 comments on commit 62a5e98

Please sign in to comment.