From 6efafa4b0e392563d5375ec62ac6e3ac8372ec61 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Mon, 12 Sep 2022 16:35:39 -0400 Subject: [PATCH] Use import order to sort CSS in prod (#4724) * Use import order to sort CSS in prod * Adding a changeset * Pass through the parent id * Update remaining test --- .changeset/many-tables-brush.md | 5 +++++ packages/astro/src/core/build/graph.ts | 12 +++++----- packages/astro/src/core/build/internal.ts | 22 ++++++++++++++----- packages/astro/src/core/build/types.ts | 2 +- .../astro/src/core/build/vite-plugin-css.ts | 21 ++++++++++++------ packages/astro/test/css-order.test.js | 21 +++++++++++++++--- .../test/fixtures/css-order/astro.config.mjs | 14 ++++++++++++ .../fixtures/css-order/src/pages/three.astro | 14 ++++++++++++ .../fixtures/css-order/src/styles/base.css | 3 +++ 9 files changed, 93 insertions(+), 21 deletions(-) create mode 100644 .changeset/many-tables-brush.md create mode 100644 packages/astro/test/fixtures/css-order/astro.config.mjs create mode 100644 packages/astro/test/fixtures/css-order/src/pages/three.astro create mode 100644 packages/astro/test/fixtures/css-order/src/styles/base.css diff --git a/.changeset/many-tables-brush.md b/.changeset/many-tables-brush.md new file mode 100644 index 000000000..45337df3e --- /dev/null +++ b/.changeset/many-tables-brush.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Use import order to sort CSS in the build diff --git a/packages/astro/src/core/build/graph.ts b/packages/astro/src/core/build/graph.ts index 8db52a1c9..e55361d75 100644 --- a/packages/astro/src/core/build/graph.ts +++ b/packages/astro/src/core/build/graph.ts @@ -7,19 +7,21 @@ export function* walkParentInfos( id: string, ctx: { getModuleInfo: GetModuleInfo }, depth = 0, - seen = new Set() -): Generator<[ModuleInfo, number], void, unknown> { + seen = new Set(), + childId = '', +): Generator<[ModuleInfo, number, number], void, unknown> { seen.add(id); const info = ctx.getModuleInfo(id); if (info) { - yield [info, depth]; + let order = childId ? info.importedIds.indexOf(childId) : 0; + yield [info, depth, order]; } const importers = (info?.importers || []).concat(info?.dynamicImporters || []); for (const imp of importers) { if (seen.has(imp)) { continue; } - yield* walkParentInfos(imp, ctx, ++depth, seen); + yield* walkParentInfos(imp, ctx, ++depth, seen, id); } } @@ -34,7 +36,7 @@ export function moduleIsTopLevelPage(info: ModuleInfo): boolean { export function* getTopLevelPages( id: string, ctx: { getModuleInfo: GetModuleInfo } -): Generator<[ModuleInfo, number], void, unknown> { +): Generator<[ModuleInfo, number, number], void, unknown> { for (const res of walkParentInfos(id, ctx)) { if (moduleIsTopLevelPage(res[0])) { yield res; diff --git a/packages/astro/src/core/build/internal.ts b/packages/astro/src/core/build/internal.ts index 7d8485fe5..ae1c48361 100644 --- a/packages/astro/src/core/build/internal.ts +++ b/packages/astro/src/core/build/internal.ts @@ -191,14 +191,26 @@ export function sortedCSS(pageData: PageBuildData) { return Array.from(pageData.css) .sort((a, b) => { let depthA = a[1].depth, - depthB = b[1].depth; + depthB = b[1].depth, + orderA = a[1].order, + orderB = b[1].order; - if (depthA === -1) { - return -1; - } else if (depthB === -1) { + if(orderA === -1 && orderB >= 0) { return 1; + } else if(orderB === -1 && orderA >= 0) { + return -1; + } else if(orderA > orderB) { + return 1; + } else if(orderA < orderB) { + return -1; } else { - return depthA > depthB ? -1 : 1; + if (depthA === -1) { + return -1; + } else if (depthB === -1) { + return 1; + } else { + return depthA > depthB ? -1 : 1; + } } }) .map(([id]) => id); diff --git a/packages/astro/src/core/build/types.ts b/packages/astro/src/core/build/types.ts index 90da4b0ef..42a4b0716 100644 --- a/packages/astro/src/core/build/types.ts +++ b/packages/astro/src/core/build/types.ts @@ -18,7 +18,7 @@ export interface PageBuildData { component: ComponentPath; route: RouteData; moduleSpecifier: string; - css: Map; + css: Map; hoistedScript: { type: 'inline' | 'external'; value: string } | undefined; } export type AllPagesData = Record; diff --git a/packages/astro/src/core/build/vite-plugin-css.ts b/packages/astro/src/core/build/vite-plugin-css.ts index 28e88cf65..202bd5319 100644 --- a/packages/astro/src/core/build/vite-plugin-css.ts +++ b/packages/astro/src/core/build/vite-plugin-css.ts @@ -110,7 +110,7 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] importedCss: Set; }; - const appendCSSToPage = (pageData: PageBuildData, meta: ViteMetadata, depth: number) => { + const appendCSSToPage = (pageData: PageBuildData, meta: ViteMetadata, depth: number, order: number) => { for (const importedCssImport of meta.importedCss) { // CSS is prioritized based on depth. Shared CSS has a higher depth due to being imported by multiple pages. // Depth info is used when sorting the links on the page. @@ -120,8 +120,15 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] if (depth < cssInfo.depth) { cssInfo.depth = depth; } + + // Update the order, preferring the lowest order we have. + if(cssInfo.order === -1) { + cssInfo.order = order; + } else if(order < cssInfo.order && order > -1) { + cssInfo.order = order; + } } else { - pageData?.css.set(importedCssImport, { depth }); + pageData?.css.set(importedCssImport, { depth, order }); } } }; @@ -161,7 +168,7 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] for (const id of Object.keys(c.modules)) { for (const pageData of getParentClientOnlys(id, this)) { for (const importedCssImport of meta.importedCss) { - pageData.css.set(importedCssImport, { depth: -1 }); + pageData.css.set(importedCssImport, { depth: -1, order: -1 }); } } } @@ -169,12 +176,12 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] // For this CSS chunk, walk parents until you find a page. Add the CSS to that page. for (const id of Object.keys(c.modules)) { - for (const [pageInfo, depth] of walkParentInfos(id, this)) { + for (const [pageInfo, depth, order] of walkParentInfos(id, this)) { if (moduleIsTopLevelPage(pageInfo)) { const pageViteID = pageInfo.id; const pageData = getPageDataByViteID(internals, pageViteID); if (pageData) { - appendCSSToPage(pageData, meta, depth); + appendCSSToPage(pageData, meta, depth, order); } } else if ( options.target === 'client' && @@ -184,7 +191,7 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] internals, pageInfo.id )) { - appendCSSToPage(pageData, meta, -1); + appendCSSToPage(pageData, meta, -1, order); } } } @@ -211,7 +218,7 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] ); if (cssChunk) { for (const pageData of eachPageData(internals)) { - pageData.css.set(cssChunk.fileName, { depth: -1 }); + pageData.css.set(cssChunk.fileName, { depth: -1, order: -1 }); } } } diff --git a/packages/astro/test/css-order.test.js b/packages/astro/test/css-order.test.js index 02db6b449..944452e93 100644 --- a/packages/astro/test/css-order.test.js +++ b/packages/astro/test/css-order.test.js @@ -86,10 +86,25 @@ describe('CSS production ordering', () => { getLinks(html).map((href) => getLinkContent(fixture, href)) ); - expect(content).to.have.a.lengthOf(2, 'there are 2 stylesheets'); - const [, last] = content; + expect(content).to.have.a.lengthOf(3, 'there are 3 stylesheets'); + const [,found] = content; - expect(last.css).to.match(/#00f/); + expect(found.css).to.match(/#00f/); + }); + + it('CSS injected by injectScript comes first because of import order', async () => { + let oneHtml = await fixture.readFile('/one/index.html'); + let twoHtml = await fixture.readFile('/two/index.html'); + let threeHtml = await fixture.readFile('/three/index.html'); + + for(const html of [oneHtml, twoHtml, threeHtml]) { + const content = await Promise.all( + getLinks(html).map((href) => getLinkContent(fixture, href)) + ); + + const [first] = content; + expect(first.css).to.include('green', 'Came from the injected script'); + } }); }); }); diff --git a/packages/astro/test/fixtures/css-order/astro.config.mjs b/packages/astro/test/fixtures/css-order/astro.config.mjs new file mode 100644 index 000000000..b53e3c0ee --- /dev/null +++ b/packages/astro/test/fixtures/css-order/astro.config.mjs @@ -0,0 +1,14 @@ +import { defineConfig } from 'astro/config'; + +export default defineConfig({ + integrations: [ + { + name: 'test-integration', + hooks: { + 'astro:config:setup'({ injectScript }) { + injectScript('page-ssr', `import '/src/styles/base.css';`); + } + } + } + ] +}); diff --git a/packages/astro/test/fixtures/css-order/src/pages/three.astro b/packages/astro/test/fixtures/css-order/src/pages/three.astro new file mode 100644 index 000000000..30289168a --- /dev/null +++ b/packages/astro/test/fixtures/css-order/src/pages/three.astro @@ -0,0 +1,14 @@ + + + Testing + + + +

Testing

+ + diff --git a/packages/astro/test/fixtures/css-order/src/styles/base.css b/packages/astro/test/fixtures/css-order/src/styles/base.css new file mode 100644 index 000000000..828bff206 --- /dev/null +++ b/packages/astro/test/fixtures/css-order/src/styles/base.css @@ -0,0 +1,3 @@ +body { + background: green; +}