summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Matthew Phillips <matthew@skypack.dev> 2022-08-23 13:26:49 -0400
committerGravatar GitHub <noreply@github.com> 2022-08-23 13:26:49 -0400
commit27ac6a03a1b58da836190922304de5645854b49b (patch)
tree587cea2084a32fc3b07a5f1fbbf69731eb71e478
parentadb20797962c280d4d38f335f577fd52a1b48d4b (diff)
downloadastro-27ac6a03a1b58da836190922304de5645854b49b.tar.gz
astro-27ac6a03a1b58da836190922304de5645854b49b.tar.zst
astro-27ac6a03a1b58da836190922304de5645854b49b.zip
Use CSS depth to sort CSS in production (#4446)
* Use CSS depth to sort CSS in production * Adds a changeset
-rw-r--r--.changeset/nine-pants-leave.md9
-rw-r--r--packages/astro/src/core/build/generate.ts6
-rw-r--r--packages/astro/src/core/build/graph.ts15
-rw-r--r--packages/astro/src/core/build/internal.ts21
-rw-r--r--packages/astro/src/core/build/page-data.ts4
-rw-r--r--packages/astro/src/core/build/types.ts2
-rw-r--r--packages/astro/src/core/build/vite-plugin-analyzer.ts4
-rw-r--r--packages/astro/src/core/build/vite-plugin-css.ts30
-rw-r--r--packages/astro/src/core/build/vite-plugin-ssr.ts4
-rw-r--r--packages/astro/test/css-order.test.js114
-rw-r--r--packages/astro/test/fixtures/css-order/src/pages/two.astro1
11 files changed, 147 insertions, 63 deletions
diff --git a/.changeset/nine-pants-leave.md b/.changeset/nine-pants-leave.md
new file mode 100644
index 000000000..75b180263
--- /dev/null
+++ b/.changeset/nine-pants-leave.md
@@ -0,0 +1,9 @@
+---
+'astro': patch
+---
+
+Deterministic CSS ordering
+
+This makes our CSS link order deterministic. It uses CSS depth; that is how deeply a module import the CSS comes from, in order to determine which CSS is page-level vs. component-level CSS.
+
+This is intended to match dev ordering where, because we do not bundle, the page-level CSS always comes after component-level.
diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts
index 68da189fa..5a32122a7 100644
--- a/packages/astro/src/core/build/generate.ts
+++ b/packages/astro/src/core/build/generate.ts
@@ -28,7 +28,7 @@ import { createRequest } from '../request.js';
import { matchRoute } from '../routing/match.js';
import { getOutputFilename } from '../util.js';
import { getOutFile, getOutFolder } from './common.js';
-import { eachPageData, getPageDataByComponent } from './internal.js';
+import { eachPageData, getPageDataByComponent, sortedCSS } from './internal.js';
import type { PageBuildData, SingleFileBuiltModule, StaticBuildOptions } from './types';
import { getTimeStat } from './util.js';
@@ -124,7 +124,7 @@ async function generatePage(
const renderers = ssrEntry.renderers;
const pageInfo = getPageDataByComponent(internals, pageData.route.component);
- const linkIds: string[] = Array.from(pageInfo?.css ?? []);
+ const linkIds: string[] = sortedCSS(pageData);
const scripts = pageInfo?.hoistedScript ?? null;
const pageModule = ssrEntry.pageMap.get(pageData.component);
@@ -264,7 +264,7 @@ async function generatePath(
astroConfig.base !== '/'
? joinPaths(astroConfig.site?.toString() || 'http://localhost/', astroConfig.base)
: astroConfig.site;
- const links = createLinkStylesheetElementSet(linkIds.reverse(), site);
+ const links = createLinkStylesheetElementSet(linkIds, site);
const scripts = createModuleScriptsSet(hoistedScripts ? [hoistedScripts] : [], site);
if (astroConfig._ctx.scripts.some((script) => script.stage === 'page')) {
diff --git a/packages/astro/src/core/build/graph.ts b/packages/astro/src/core/build/graph.ts
index e112845ab..beef79563 100644
--- a/packages/astro/src/core/build/graph.ts
+++ b/packages/astro/src/core/build/graph.ts
@@ -5,19 +5,20 @@ import { resolvedPagesVirtualModuleId } from '../app/index.js';
export function* walkParentInfos(
id: string,
ctx: { getModuleInfo: GetModuleInfo },
+ depth = 0,
seen = new Set<string>()
-): Generator<ModuleInfo, void, unknown> {
+): Generator<[ModuleInfo, number], void, unknown> {
seen.add(id);
const info = ctx.getModuleInfo(id);
if (info) {
- yield info;
+ yield [info, depth];
}
const importers = (info?.importers || []).concat(info?.dynamicImporters || []);
for (const imp of importers) {
if (seen.has(imp)) {
continue;
}
- yield* walkParentInfos(imp, ctx, seen);
+ yield* walkParentInfos(imp, ctx, ++depth, seen);
}
}
@@ -26,10 +27,10 @@ export function* walkParentInfos(
export function* getTopLevelPages(
id: string,
ctx: { getModuleInfo: GetModuleInfo }
-): Generator<ModuleInfo, void, unknown> {
- for (const info of walkParentInfos(id, ctx)) {
- if (info?.importers[0] === resolvedPagesVirtualModuleId) {
- yield info;
+): Generator<[ModuleInfo, number], void, unknown> {
+ for (const res of walkParentInfos(id, ctx)) {
+ if (res[0]?.importers[0] === resolvedPagesVirtualModuleId) {
+ yield res;
}
}
}
diff --git a/packages/astro/src/core/build/internal.ts b/packages/astro/src/core/build/internal.ts
index 2926e8270..d7db27069 100644
--- a/packages/astro/src/core/build/internal.ts
+++ b/packages/astro/src/core/build/internal.ts
@@ -181,3 +181,24 @@ export function hasPageDataByViteID(internals: BuildInternals, viteid: ViteID):
export function* eachPageData(internals: BuildInternals) {
yield* internals.pagesByComponent.values();
}
+
+/**
+ * Sort a page's CSS by depth. A higher depth means that the CSS comes from shared subcomponents.
+ * A lower depth means it comes directly from the top-level page.
+ * The return of this function is an array of CSS paths, with shared CSS on top
+ * and page-level CSS on bottom.
+ */
+export function sortedCSS(pageData: PageBuildData) {
+ return Array.from(pageData.css).sort((a, b) => {
+ let depthA = a[1].depth,
+ depthB = b[1].depth;
+
+ 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/page-data.ts b/packages/astro/src/core/build/page-data.ts
index 5de8804c4..509d1ae20 100644
--- a/packages/astro/src/core/build/page-data.ts
+++ b/packages/astro/src/core/build/page-data.ts
@@ -53,7 +53,7 @@ export async function collectPagesData(
component: route.component,
route,
moduleSpecifier: '',
- css: new Set(),
+ css: new Map(),
hoistedScript: undefined,
};
@@ -74,7 +74,7 @@ export async function collectPagesData(
component: route.component,
route,
moduleSpecifier: '',
- css: new Set(),
+ css: new Map(),
hoistedScript: undefined,
};
}
diff --git a/packages/astro/src/core/build/types.ts b/packages/astro/src/core/build/types.ts
index 941ec35f6..90da4b0ef 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: Set<string>;
+ css: Map<string, { depth: number }>;
hoistedScript: { type: 'inline' | 'external'; value: string } | undefined;
}
export type AllPagesData = Record<ComponentPath, PageBuildData>;
diff --git a/packages/astro/src/core/build/vite-plugin-analyzer.ts b/packages/astro/src/core/build/vite-plugin-analyzer.ts
index a859bcd7c..e4f64b181 100644
--- a/packages/astro/src/core/build/vite-plugin-analyzer.ts
+++ b/packages/astro/src/core/build/vite-plugin-analyzer.ts
@@ -22,7 +22,7 @@ export function vitePluginAnalyzer(internals: BuildInternals): VitePlugin {
}
if (hoistedScripts.size) {
- for (const pageInfo of getTopLevelPages(from, this)) {
+ for (const [pageInfo] of getTopLevelPages(from, this)) {
const pageId = pageInfo.id;
for (const hid of hoistedScripts) {
if (pageScripts.has(pageId)) {
@@ -98,7 +98,7 @@ export function vitePluginAnalyzer(internals: BuildInternals): VitePlugin {
clientOnlys.push(cid);
}
- for (const pageInfo of getTopLevelPages(id, this)) {
+ for (const [pageInfo] of getTopLevelPages(id, this)) {
const pageData = getPageDataByViteID(internals, pageInfo.id);
if (!pageData) continue;
diff --git a/packages/astro/src/core/build/vite-plugin-css.ts b/packages/astro/src/core/build/vite-plugin-css.ts
index 8b2a31005..1788d9ee8 100644
--- a/packages/astro/src/core/build/vite-plugin-css.ts
+++ b/packages/astro/src/core/build/vite-plugin-css.ts
@@ -42,8 +42,8 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
}
function createNameForParentPages(id: string, ctx: { getModuleInfo: GetModuleInfo }): string {
- const parents = Array.from(getTopLevelPages(id, ctx)).sort();
- const proposedName = parents.map((page) => nameifyPage(page.id)).join('-');
+ const parents = Array.from(getTopLevelPages(id, ctx));
+ const proposedName = parents.map(([page]) => nameifyPage(page.id)).sort().join('-');
// We don't want absurdedly long chunk names, so if this is too long create a hashed version instead.
if (proposedName.length <= MAX_NAME_LENGTH) {
@@ -51,7 +51,7 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
}
const hash = crypto.createHash('sha256');
- for (const page of parents) {
+ for (const [page] of parents) {
hash.update(page.id, 'utf-8');
}
return hash.digest('hex').slice(0, 8);
@@ -61,7 +61,7 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
id: string,
ctx: { getModuleInfo: GetModuleInfo }
): Generator<PageBuildData, void, unknown> {
- for (const info of walkParentInfos(id, ctx)) {
+ for (const [info] of walkParentInfos(id, ctx)) {
yield* getPageDatasByClientOnlyID(internals, info.id);
}
}
@@ -107,6 +107,10 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
// Chunks that have the viteMetadata.importedCss are CSS chunks
if (meta.importedCss.size) {
+ //console.log(meta.importedCss, c.fileName);
+
+ debugger;
+
// For the client build, client:only styles need to be mapped
// over to their page. For this chunk, determine if it's a child of a
// client:only component and if so, add its CSS to the page it belongs to.
@@ -114,7 +118,7 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
for (const [id] of Object.entries(c.modules)) {
for (const pageData of getParentClientOnlys(id, this)) {
for (const importedCssImport of meta.importedCss) {
- pageData.css.add(importedCssImport);
+ pageData.css.set(importedCssImport, { depth: -1 });
}
}
}
@@ -122,11 +126,23 @@ 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.entries(c.modules)) {
- for (const pageInfo of getTopLevelPages(id, this)) {
+ debugger;
+ for (const [pageInfo, depth] of getTopLevelPages(id, this)) {
const pageViteID = pageInfo.id;
const pageData = getPageDataByViteID(internals, pageViteID);
+
for (const importedCssImport of meta.importedCss) {
- pageData?.css.add(importedCssImport);
+ // 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.
+ if(pageData?.css.has(importedCssImport)) {
+ // eslint-disable-next-line
+ const cssInfo = pageData?.css.get(importedCssImport)!;
+ if(depth < cssInfo.depth) {
+ cssInfo.depth = depth;
+ }
+ } else {
+ pageData?.css.set(importedCssImport, { depth });
+ }
}
}
}
diff --git a/packages/astro/src/core/build/vite-plugin-ssr.ts b/packages/astro/src/core/build/vite-plugin-ssr.ts
index 7e373d0ea..71a58bfe4 100644
--- a/packages/astro/src/core/build/vite-plugin-ssr.ts
+++ b/packages/astro/src/core/build/vite-plugin-ssr.ts
@@ -12,7 +12,7 @@ import { BEFORE_HYDRATION_SCRIPT_ID, PAGE_SCRIPT_ID } from '../../vite-plugin-sc
import { pagesVirtualModuleId } from '../app/index.js';
import { serializeRouteData } from '../routing/index.js';
import { addRollupInput } from './add-rollup-input.js';
-import { eachPageData } from './internal.js';
+import { eachPageData, sortedCSS } from './internal.js';
export const virtualModuleId = '@astrojs-ssr-virtual-entry';
const resolvedVirtualModuleId = '\0' + virtualModuleId;
@@ -139,7 +139,7 @@ function buildManifest(
routes.push({
file: '',
- links: Array.from(pageData.css).reverse(),
+ links: sortedCSS(pageData),
scripts: [
...scripts,
...astroConfig._ctx.scripts
diff --git a/packages/astro/test/css-order.test.js b/packages/astro/test/css-order.test.js
index 4477d4660..402e6b619 100644
--- a/packages/astro/test/css-order.test.js
+++ b/packages/astro/test/css-order.test.js
@@ -4,13 +4,6 @@ import { loadFixture } from './test-utils.js';
import testAdapter from './test-adapter.js';
describe('CSS production ordering', () => {
- let staticHTML, serverHTML;
- let staticCSS, serverCSS;
-
- const commonConfig = Object.freeze({
- root: './fixtures/css-order/',
- });
-
function getLinks(html) {
let $ = cheerio.load(html);
let out = [];
@@ -20,42 +13,85 @@ describe('CSS production ordering', () => {
return out;
}
- before(async () => {
- let fixture = await loadFixture({ ...commonConfig });
- await fixture.build();
- staticHTML = await fixture.readFile('/one/index.html');
- staticCSS = await Promise.all(
- getLinks(staticHTML).map(async (href) => {
- const css = await fixture.readFile(href);
- return { href, css };
- })
- );
- });
+ /**
+ *
+ * @param {import('./test-utils').Fixture} fixture
+ * @param {string} href
+ * @returns {Promise<{ href: string; css: string; }>}
+ */
+ async function getLinkContent(fixture, href) {
+ const css = await fixture.readFile(href);
+ return { href, css };
+ }
+
+ describe('SSG and SSR parity', () => {
+ let staticHTML, serverHTML;
+ let staticCSS, serverCSS;
+
+ const commonConfig = Object.freeze({
+ root: './fixtures/css-order/',
+ });
+
- before(async () => {
- let fixture = await loadFixture({
- ...commonConfig,
- adapter: testAdapter(),
- output: 'server',
+
+ before(async () => {
+ let fixture = await loadFixture({ ...commonConfig });
+ await fixture.build();
+ staticHTML = await fixture.readFile('/one/index.html');
+ staticCSS = await Promise.all(
+ getLinks(staticHTML).map(href => getLinkContent(fixture, href))
+ );
+ });
+
+ before(async () => {
+ let fixture = await loadFixture({
+ ...commonConfig,
+ adapter: testAdapter(),
+ output: 'server',
+ });
+ await fixture.build();
+
+ const app = await fixture.loadTestAdapterApp();
+ const request = new Request('http://example.com/one');
+ const response = await app.render(request);
+ serverHTML = await response.text();
+ serverCSS = await Promise.all(
+ getLinks(serverHTML).map(async (href) => {
+ const css = await fixture.readFile(`/client${href}`);
+ return { href, css };
+ })
+ );
+ });
+
+ it('is in the same order for output: server and static', async () => {
+ const staticContent = staticCSS.map((o) => o.css);
+ const serverContent = serverCSS.map((o) => o.css);
+
+ expect(staticContent).to.deep.equal(serverContent);
});
- await fixture.build();
-
- const app = await fixture.loadTestAdapterApp();
- const request = new Request('http://example.com/one');
- const response = await app.render(request);
- serverHTML = await response.text();
- serverCSS = await Promise.all(
- getLinks(serverHTML).map(async (href) => {
- const css = await fixture.readFile(`/client${href}`);
- return { href, css };
- })
- );
});
- it('is in the same order for output: server and static', async () => {
- const staticContent = staticCSS.map((o) => o.css);
- const serverContent = serverCSS.map((o) => o.css);
+ describe('Page vs. Shared CSS', () => {
+ /** @type {import('./test-utils').Fixture} */
+ let fixture;
+ before(async () => {
+ fixture = await loadFixture({
+ root: './fixtures/css-order/',
+ });
+ await fixture.build();
+ });
+
+ it('Page level CSS is defined lower in the page', async () => {
+ let html = await fixture.readFile('/two/index.html');
- expect(staticContent).to.deep.equal(serverContent);
+ const content = await Promise.all(
+ getLinks(html).map(href => getLinkContent(fixture, href))
+ );
+
+ expect(content).to.have.a.lengthOf(2, 'there are 2 stylesheets');
+ const [,last] = content;
+
+ expect(last.css).to.match(/#00f/);
+ });
});
});
diff --git a/packages/astro/test/fixtures/css-order/src/pages/two.astro b/packages/astro/test/fixtures/css-order/src/pages/two.astro
index 46d899bed..3652cb013 100644
--- a/packages/astro/test/fixtures/css-order/src/pages/two.astro
+++ b/packages/astro/test/fixtures/css-order/src/pages/two.astro
@@ -8,6 +8,7 @@ import CommonHead from '../components/CommonHead.astro';
<style>
body {
margin: 2px;
+ color: blue;
}
</style>
</head>