Enforce consistent import order of CSS (#2065)

Partially fixes #2060
withastro · Dec 1, 2021 · c6e4e28 · c6e4e28
1 parent 754ac42
commit c6e4e28
Show file tree

Hide file tree

Showing 11 changed files with 134 additions and 11 deletions.
diff --git a/.changeset/bright-ravens-remain.md b/.changeset/bright-ravens-remain.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Bugfix: improve CSS import order
diff --git a/packages/astro/src/vite-plugin-build-css/index.ts b/packages/astro/src/vite-plugin-build-css/index.ts
@@ -94,25 +94,21 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin {
 
  async load(id) {
  if (isPageStyleVirtualModule(id)) {
- const source = astroPageStyleMap.get(id)!;
- return source;
+ return astroPageStyleMap.get(id) || null;
  }
  if (isStyleVirtualModule(id)) {
- return astroStyleMap.get(id)!;
+ return astroStyleMap.get(id) || null;
  }
  return null;
  },
 
  async transform(value, id) {
  if (isStyleVirtualModule(id)) {
  styleSourceMap.set(id, value);
- return null;
  }
-
  if (isCSSRequest(id)) {
  styleSourceMap.set(id, value);
  }
-
  return null;
  },
 

diff --git a/packages/astro/src/vite-plugin-build-html/index.ts b/packages/astro/src/vite-plugin-build-html/index.ts
@@ -256,11 +256,10 @@ export function rollupPluginAstroBuildHTML(options: PluginOptions): VitePlugin {
  // Create a mapping of chunks to dependent chunks, used to add the proper
  // link tags for CSS.
  for (const chunk of pureCSSChunks) {
- const referenceId = chunkToReferenceIdMap.get(chunk.fileName)!;
- const chunkReferenceIds = [referenceId];
- for (const imp of chunk.imports) {
- if (chunkToReferenceIdMap.has(imp)) {
- chunkReferenceIds.push(chunkToReferenceIdMap.get(imp)!);
+ const chunkReferenceIds: string[] = [];
+ for (const [specifier, chunkRefID] of chunkToReferenceIdMap.entries()) {
+ if (chunk.imports.includes(specifier) || specifier === chunk.fileName) {
+ chunkReferenceIds.push(chunkRefID);
  }
  }
  for (const [id] of Object.entries(chunk.modules)) {

diff --git a/packages/astro/test/astro-css-bundling-import.test.js b/packages/astro/test/astro-css-bundling-import.test.js
@@ -0,0 +1,50 @@
+import { expect } from 'chai';
+import cheerio from 'cheerio';
+import { loadFixture } from './test-utils.js';
+
+describe('CSS Bundling (ESM import)', () => {
+ let fixture;
+
+ before(async () => {
+ fixture = await loadFixture({ projectRoot: './fixtures/astro-css-bundling-import/' });
+ await fixture.build();
+ });
+
+ 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.
+ const html = await fixture.readFile('/page-2/index.html');
+ const $ = cheerio.load(html);
+
+ let css = '';
+
+ for (const style of $('link[rel=stylesheet]')) {
+ const href = style.attribs.href.replace(/^\.\./, '');
+ if (!href) continue;
+ css += await fixture.readFile(href);
+ }
+
+ // test 1: insure green comes after red (site.css)
+ 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}'));
+ });
+
+ // TODO: need more investigation to fix this
+ it.skip('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);
+
+ for (const style of $('link[rel=stylesheet]')) {
+ const href = style.attribs.href.replace(/^\.\./, '');
+ if (!href) continue;
+ const css = await fixture.readFile(href);
+
+ expect(css).to.be.ok;
+ }
+ }
+ });
+});
diff --git a/packages/astro/test/fixtures/astro-css-bundling-import/src/layouts/BaseLayout.astro b/packages/astro/test/fixtures/astro-css-bundling-import/src/layouts/BaseLayout.astro
@@ -0,0 +1,27 @@
+---
+import "../styles/site.css"
+
+const {title} = Astro.props;
+---
+
+<html lang="en">
+
+<head>
+ <meta charset="utf-8" />
+ <link rel="icon" type="image/x-icon" href="/favicon.ico" />
+ <meta name="viewport" content="width=device-width" />
+ <title>{title}</title>
+ <style>
+ </style>
+</head>
+
+<body>
+ <ul>
+ <li><a href="/page-1">Page 1</a></li>
+ <li><a href="/page-1">Page 2</a></li>
+ <!-- <li><a href="/page-2-reduced-layout">Page 2 reduced layout</a></li> -->
+ </ul>
+ <slot></slot>
+</body>
+
+</html>
diff --git a/packages/astro/test/fixtures/astro-css-bundling-import/src/layouts/PageLayout.astro b/packages/astro/test/fixtures/astro-css-bundling-import/src/layouts/PageLayout.astro
@@ -0,0 +1,12 @@
+---
+import BaseLayout from "./BaseLayout.astro"
+import "../styles/page-one.css"
+
+const {title} = Astro.props;
+---
+
+<BaseLayout title={title}>
+ <main id="page">
+ <slot></slot>
+ </main>
+</BaseLayout>
diff --git a/packages/astro/test/fixtures/astro-css-bundling-import/src/pages/page-1.astro b/packages/astro/test/fixtures/astro-css-bundling-import/src/pages/page-1.astro
@@ -0,0 +1,8 @@
+---
+import PageLayout from "../layouts/PageLayout.astro"
+---
+
+<PageLayout title="Page 1">
+ <h1>Page 1</h1>
+ <p>Nothing to see here. Check <a href="/page-2">Page 2</a></p>
+</PageLayout>
diff --git a/packages/astro/test/fixtures/astro-css-bundling-import/src/pages/page-2.astro b/packages/astro/test/fixtures/astro-css-bundling-import/src/pages/page-2.astro
@@ -0,0 +1,13 @@
+---
+import PageLayout from "../layouts/PageLayout.astro"
+import "../styles/page-two.css"
+---
+
+<PageLayout title="Page 2">
+ <h1>Page 2</h1>
+ <p>This text should be green, because we want <code>page-2.css</code> to override <code>site.css</code></p>
+ <p>This works in the dev-server. However in the prod build, the text is blue. Execute <code>npm run build</code> and then execute <code>npx http-server dist/</code>.</p>
+ <p>We can view the built html at <a href="https://github-qoihup--8080.local.webcontainer.io/page-2/">https://github-qoihup--8080.local.webcontainer.io/page-2/</a>. The color there is blue.</p>
+
+ <p>If it helps debug the issue, rename the <code>page-1.astro</code> file to <code>page-1.astro.bak</code>. Build the prod site and view it. This time the color is green.</p>
+</PageLayout>
diff --git a/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/page-one.css b/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/page-one.css
@@ -0,0 +1,3 @@
+p {
+ color: blue;
+}
diff --git a/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/page-two.css b/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/page-two.css
@@ -0,0 +1,3 @@
+p {
+ color: green;
+}
diff --git a/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/site.css b/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/site.css
@@ -0,0 +1,7 @@
+p {
+ color: red;
+}
+
+h1 {
+ outline: 1px solid red;
+}