From 795f00f73c549727e05d5b7bf0e39cce87add4e7 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Tue, 6 Dec 2022 17:12:25 -0500 Subject: [PATCH] Use accumulated sort order when order production CSS (#5549) * Use accumulated sort order when order production CSS * Adding a changeset * Fix lockfile issue --- .changeset/good-terms-walk.md | 5 ++ packages/astro/src/core/build/graph.ts | 7 ++- packages/astro/test/css-order-layout.test.js | 63 +++++++++++++++++++ .../css-order-layout/astro.config.mjs | 3 + .../fixtures/css-order-layout/package.json | 6 ++ .../src/components/BlueButton.astro | 17 +++++ .../src/components/Button.astro | 8 +++ .../src/components/MainHead.astro | 5 ++ .../css-order-layout/src/layouts/Main.astro | 14 +++++ .../css-order-layout/src/layouts/Second.astro | 14 +++++ .../css-order-layout/src/pages/admin.astro | 9 +++ .../css-order-layout/src/pages/beta.astro | 11 ++++ .../css-order-layout/src/pages/index.astro | 11 ++++ .../css-order-layout/src/styles/global.css | 3 + pnpm-lock.yaml | 6 ++ 15 files changed, 180 insertions(+), 2 deletions(-) create mode 100644 .changeset/good-terms-walk.md create mode 100644 packages/astro/test/css-order-layout.test.js create mode 100644 packages/astro/test/fixtures/css-order-layout/astro.config.mjs create mode 100644 packages/astro/test/fixtures/css-order-layout/package.json create mode 100644 packages/astro/test/fixtures/css-order-layout/src/components/BlueButton.astro create mode 100644 packages/astro/test/fixtures/css-order-layout/src/components/Button.astro create mode 100644 packages/astro/test/fixtures/css-order-layout/src/components/MainHead.astro create mode 100644 packages/astro/test/fixtures/css-order-layout/src/layouts/Main.astro create mode 100644 packages/astro/test/fixtures/css-order-layout/src/layouts/Second.astro create mode 100644 packages/astro/test/fixtures/css-order-layout/src/pages/admin.astro create mode 100644 packages/astro/test/fixtures/css-order-layout/src/pages/beta.astro create mode 100644 packages/astro/test/fixtures/css-order-layout/src/pages/index.astro create mode 100644 packages/astro/test/fixtures/css-order-layout/src/styles/global.css diff --git a/.changeset/good-terms-walk.md b/.changeset/good-terms-walk.md new file mode 100644 index 0000000000..c6d1c0d893 --- /dev/null +++ b/.changeset/good-terms-walk.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Use accumulated sort order when order production CSS diff --git a/packages/astro/src/core/build/graph.ts b/packages/astro/src/core/build/graph.ts index a667ac4282..d78c9baa41 100644 --- a/packages/astro/src/core/build/graph.ts +++ b/packages/astro/src/core/build/graph.ts @@ -7,13 +7,16 @@ export function* walkParentInfos( id: string, ctx: { getModuleInfo: GetModuleInfo }, depth = 0, + order = 0, seen = new Set(), childId = '' ): Generator<[ModuleInfo, number, number], void, unknown> { seen.add(id); const info = ctx.getModuleInfo(id); if (info) { - let order = childId ? info.importedIds.indexOf(childId) : 0; + if(childId) { + order += info.importedIds.indexOf(childId) + } yield [info, depth, order]; } const importers = (info?.importers || []).concat(info?.dynamicImporters || []); @@ -21,7 +24,7 @@ export function* walkParentInfos( if (seen.has(imp)) { continue; } - yield* walkParentInfos(imp, ctx, ++depth, seen, id); + yield* walkParentInfos(imp, ctx, ++depth, order, seen, id); } } diff --git a/packages/astro/test/css-order-layout.test.js b/packages/astro/test/css-order-layout.test.js new file mode 100644 index 0000000000..5df2d0b0f7 --- /dev/null +++ b/packages/astro/test/css-order-layout.test.js @@ -0,0 +1,63 @@ +import { expect } from 'chai'; +import * as cheerio from 'cheerio'; +import { loadFixture } from './test-utils.js'; + +describe('CSS ordering - import order with layouts', () => { + /** @type {import('./test-utils').Fixture} */ + let fixture; + before(async () => { + fixture = await loadFixture({ + root: './fixtures/css-order-layout/', + }); + }); + + /** + * + * @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; + } + + /** + * + * @param {string} href + * @returns {Promise<{ href: string; css: string; }>} + */ + async function getLinkContent(href) { + const css = await fixture.readFile(href); + return { href, css }; + } + + 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))); + + let specialButtonCSS = -1; + let globalCSS = -1; + for(let i = 0; i < content.length; i++) { + if(content[i].css.indexOf('.SpecialButton') !== -1) { + specialButtonCSS = i; + } else if(content[i].css.indexOf('green') !== -1) { + globalCSS = i; + } + } + + expect(globalCSS).to.equal(0, 'global css sorted on top') + expect(specialButtonCSS).to.equal(1, 'component level css sorted last'); + }); + }); +}); diff --git a/packages/astro/test/fixtures/css-order-layout/astro.config.mjs b/packages/astro/test/fixtures/css-order-layout/astro.config.mjs new file mode 100644 index 0000000000..ef0fcdd507 --- /dev/null +++ b/packages/astro/test/fixtures/css-order-layout/astro.config.mjs @@ -0,0 +1,3 @@ +import { defineConfig } from "astro/config"; + +export default defineConfig({}); diff --git a/packages/astro/test/fixtures/css-order-layout/package.json b/packages/astro/test/fixtures/css-order-layout/package.json new file mode 100644 index 0000000000..880c27ca87 --- /dev/null +++ b/packages/astro/test/fixtures/css-order-layout/package.json @@ -0,0 +1,6 @@ +{ + "name": "@test/css-order-layout", + "dependencies": { + "astro": "workspace:*" + } +} diff --git a/packages/astro/test/fixtures/css-order-layout/src/components/BlueButton.astro b/packages/astro/test/fixtures/css-order-layout/src/components/BlueButton.astro new file mode 100644 index 0000000000..05dcd793fc --- /dev/null +++ b/packages/astro/test/fixtures/css-order-layout/src/components/BlueButton.astro @@ -0,0 +1,17 @@ +--- +import Button from "./Button.astro"; + +/* +TODO: +- Improve keyboard navigation / focus handling ref: https://w3c.github.io/aria-practices/examples/disclosure/disclosure-navigation.html + */ +--- + + + +
This button should be blue
+ diff --git a/packages/astro/test/fixtures/css-order-layout/src/components/MainHead.astro b/packages/astro/test/fixtures/css-order-layout/src/components/MainHead.astro new file mode 100644 index 0000000000..c87e713bcc --- /dev/null +++ b/packages/astro/test/fixtures/css-order-layout/src/components/MainHead.astro @@ -0,0 +1,5 @@ +--- +import "../styles/global.css"; +--- + + diff --git a/packages/astro/test/fixtures/css-order-layout/src/layouts/Main.astro b/packages/astro/test/fixtures/css-order-layout/src/layouts/Main.astro new file mode 100644 index 0000000000..49f378bc64 --- /dev/null +++ b/packages/astro/test/fixtures/css-order-layout/src/layouts/Main.astro @@ -0,0 +1,14 @@ +--- +import MainHead from "../components/MainHead.astro"; +import BlueButton from "../components/BlueButton.astro"; +--- + + + + + + + + + + diff --git a/packages/astro/test/fixtures/css-order-layout/src/layouts/Second.astro b/packages/astro/test/fixtures/css-order-layout/src/layouts/Second.astro new file mode 100644 index 0000000000..49f378bc64 --- /dev/null +++ b/packages/astro/test/fixtures/css-order-layout/src/layouts/Second.astro @@ -0,0 +1,14 @@ +--- +import MainHead from "../components/MainHead.astro"; +import BlueButton from "../components/BlueButton.astro"; +--- + + + + + + + + + + diff --git a/packages/astro/test/fixtures/css-order-layout/src/pages/admin.astro b/packages/astro/test/fixtures/css-order-layout/src/pages/admin.astro new file mode 100644 index 0000000000..e35c3112d8 --- /dev/null +++ b/packages/astro/test/fixtures/css-order-layout/src/pages/admin.astro @@ -0,0 +1,9 @@ +--- +import MainHead from "../components/MainHead.astro"; +--- + + + + + + diff --git a/packages/astro/test/fixtures/css-order-layout/src/pages/beta.astro b/packages/astro/test/fixtures/css-order-layout/src/pages/beta.astro new file mode 100644 index 0000000000..f9ba42b470 --- /dev/null +++ b/packages/astro/test/fixtures/css-order-layout/src/pages/beta.astro @@ -0,0 +1,11 @@ +--- +import SecondLayout from "../layouts/Second.astro"; +--- + + + +
+ Subpage (Home) +
+ + diff --git a/packages/astro/test/fixtures/css-order-layout/src/pages/index.astro b/packages/astro/test/fixtures/css-order-layout/src/pages/index.astro new file mode 100644 index 0000000000..a565d6e715 --- /dev/null +++ b/packages/astro/test/fixtures/css-order-layout/src/pages/index.astro @@ -0,0 +1,11 @@ +--- +import MainLayout from "../layouts/Main.astro"; +--- + + + +
+ Home (Subpage) +
+ + diff --git a/packages/astro/test/fixtures/css-order-layout/src/styles/global.css b/packages/astro/test/fixtures/css-order-layout/src/styles/global.css new file mode 100644 index 0000000000..6b527a0940 --- /dev/null +++ b/packages/astro/test/fixtures/css-order-layout/src/styles/global.css @@ -0,0 +1,3 @@ +.btn { + color: green; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index fd205edfcd..f41745f3f6 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1670,6 +1670,12 @@ importers: dependencies: astro: link:../../.. + packages/astro/test/fixtures/css-order-layout: + specifiers: + astro: workspace:* + dependencies: + astro: link:../../.. + packages/astro/test/fixtures/custom-404: specifiers: astro: workspace:*