summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Matthew Phillips <matthew@skypack.dev> 2022-03-28 16:48:06 -0400
committerGravatar GitHub <noreply@github.com> 2022-03-28 16:48:06 -0400
commit77354c89bd606beba71231cce6ce935905de68a7 (patch)
tree58fe284c035d8deb33da0ffa2c17138b4d954854
parentc40813e932c1589f37ee6d63e2d495679c4bedb0 (diff)
downloadastro-77354c89bd606beba71231cce6ce935905de68a7.tar.gz
astro-77354c89bd606beba71231cce6ce935905de68a7.tar.zst
astro-77354c89bd606beba71231cce6ce935905de68a7.zip
Prevent CSS from being added to pages not using it (#2918)
* Prevent CSS from being added to pages not using it * Adds a changeset * Add clarification when the CSS is appended to the pageData * Move addStyles up a level
-rw-r--r--.changeset/tiny-snails-grab.md5
-rw-r--r--packages/astro/src/core/build/internal.ts4
-rw-r--r--packages/astro/src/core/build/scan-based-build.ts1
-rw-r--r--packages/astro/src/core/build/static-build.ts3
-rw-r--r--packages/astro/src/core/build/vite-plugin-pages.ts2
-rw-r--r--packages/astro/src/vite-plugin-build-css/index.ts70
-rw-r--r--packages/astro/test/fixtures/page-level-styles/src/layouts/Base.astro13
-rw-r--r--packages/astro/test/fixtures/page-level-styles/src/layouts/Styled.astro13
-rw-r--r--packages/astro/test/fixtures/page-level-styles/src/pages/blog.astro9
-rw-r--r--packages/astro/test/fixtures/page-level-styles/src/pages/index.astro9
-rw-r--r--packages/astro/test/fixtures/page-level-styles/src/styles/main.css4
-rw-r--r--packages/astro/test/page-level-styles.test.js27
12 files changed, 153 insertions, 7 deletions
diff --git a/.changeset/tiny-snails-grab.md b/.changeset/tiny-snails-grab.md
new file mode 100644
index 000000000..2f7d50651
--- /dev/null
+++ b/.changeset/tiny-snails-grab.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Prevent CSS from being added to the wrong pages
diff --git a/packages/astro/src/core/build/internal.ts b/packages/astro/src/core/build/internal.ts
index c436b9c5c..674a657b6 100644
--- a/packages/astro/src/core/build/internal.ts
+++ b/packages/astro/src/core/build/internal.ts
@@ -105,6 +105,10 @@ export function getPageDataByViteID(internals: BuildInternals, viteid: ViteID):
return undefined;
}
+export function hasPageDataByViteID(internals: BuildInternals, viteid: ViteID): boolean {
+ return internals.pagesByViteID.has(viteid);
+}
+
export function* eachPageData(internals: BuildInternals) {
yield* internals.pagesByComponent.values();
}
diff --git a/packages/astro/src/core/build/scan-based-build.ts b/packages/astro/src/core/build/scan-based-build.ts
index 1a4cf16e4..ec473ca79 100644
--- a/packages/astro/src/core/build/scan-based-build.ts
+++ b/packages/astro/src/core/build/scan-based-build.ts
@@ -76,6 +76,7 @@ export async function build(opts: ScanBasedBuildOptions): Promise<RollupOutput |
}),
rollupPluginAstroBuildCSS({
internals,
+ legacy: true
}),
...(viteConfig.plugins || []),
],
diff --git a/packages/astro/src/core/build/static-build.ts b/packages/astro/src/core/build/static-build.ts
index 2e54891d3..086d0b82e 100644
--- a/packages/astro/src/core/build/static-build.ts
+++ b/packages/astro/src/core/build/static-build.ts
@@ -132,7 +132,6 @@ async function ssrBuild(opts: StaticBuildOptions, internals: BuildInternals, inp
entryFileNames: opts.buildConfig.serverEntry,
chunkFileNames: 'chunks/chunk.[hash].mjs',
assetFileNames: 'assets/asset.[hash][extname]',
- inlineDynamicImports: true,
},
},
// must match an esbuild target
@@ -147,6 +146,7 @@ async function ssrBuild(opts: StaticBuildOptions, internals: BuildInternals, inp
vitePluginPages(opts, internals),
rollupPluginAstroBuildCSS({
internals,
+ legacy: false
}),
...(viteConfig.plugins || []),
// SSR needs to be last
@@ -200,6 +200,7 @@ async function clientBuild(opts: StaticBuildOptions, internals: BuildInternals,
vitePluginHoistedScripts(astroConfig, internals),
rollupPluginAstroBuildCSS({
internals,
+ legacy: false
}),
...(viteConfig.plugins || []),
],
diff --git a/packages/astro/src/core/build/vite-plugin-pages.ts b/packages/astro/src/core/build/vite-plugin-pages.ts
index 22991c8ae..ea7fe1600 100644
--- a/packages/astro/src/core/build/vite-plugin-pages.ts
+++ b/packages/astro/src/core/build/vite-plugin-pages.ts
@@ -6,7 +6,7 @@ import { eachPageData } from './internal.js';
import { isBuildingToSSR } from '../util.js';
export const virtualModuleId = '@astrojs-pages-virtual-entry';
-const resolvedVirtualModuleId = '\0' + virtualModuleId;
+export const resolvedVirtualModuleId = '\0' + virtualModuleId;
export function vitePluginPages(opts: StaticBuildOptions, internals: BuildInternals): VitePlugin {
return {
diff --git a/packages/astro/src/vite-plugin-build-css/index.ts b/packages/astro/src/vite-plugin-build-css/index.ts
index 7de50af4e..94be52405 100644
--- a/packages/astro/src/vite-plugin-build-css/index.ts
+++ b/packages/astro/src/vite-plugin-build-css/index.ts
@@ -1,10 +1,12 @@
-import type { BuildInternals } from '../core/build/internal';
+import { BuildInternals } from '../core/build/internal';
+import type { ModuleInfo, PluginContext } from 'rollup';
import * as path from 'path';
import esbuild from 'esbuild';
import { Plugin as VitePlugin } from 'vite';
import { isCSSRequest } from '../core/render/dev/css.js';
-import { getPageDatasByChunk } from '../core/build/internal.js';
+import { getPageDatasByChunk, getPageDataByViteID, hasPageDataByViteID } from '../core/build/internal.js';
+import { resolvedVirtualModuleId as virtualPagesModuleId } from '../core/build/vite-plugin-pages.js';
const PLUGIN_NAME = '@astrojs/rollup-plugin-build-css';
@@ -44,12 +46,64 @@ function isPageStyleVirtualModule(id: string) {
interface PluginOptions {
internals: BuildInternals;
+ legacy: boolean;
}
export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin {
- const { internals } = options;
+ const { internals, legacy } = options;
const styleSourceMap = new Map<string, string>();
+ function * walkStyles(ctx: PluginContext, id: string, seen = new Set<string>()): Generator<[string, string], void, unknown> {
+ seen.add(id);
+ if(styleSourceMap.has(id)) {
+ yield [id, styleSourceMap.get(id)!];
+ }
+
+ const info = ctx.getModuleInfo(id);
+ if(info) {
+ for(const importedId of info.importedIds) {
+ if(!seen.has(importedId)) {
+ yield * walkStyles(ctx, importedId, seen);
+ }
+ }
+ }
+ }
+
+ /**
+ * This walks the dependency graph looking for styles that are imported
+ * by a page and then creates a chunking containing all of the styles for that page.
+ * Since there is only 1 entrypoint for the entire app, we do this in order
+ * to prevent adding all styles to all pages.
+ */
+ async function addStyles(this: PluginContext) {
+ for(const id of this.getModuleIds()) {
+ if(hasPageDataByViteID(internals, id)) {
+ let pageStyles = '';
+ for(const [_styleId, styles] of walkStyles(this, id)) {
+ pageStyles += styles;
+ }
+
+ // Pages with no styles, nothing more to do
+ if(!pageStyles) continue;
+
+ const { code: minifiedCSS } = await esbuild.transform(pageStyles, {
+ loader: 'css',
+ minify: true,
+ });
+ const referenceId = this.emitFile({
+ name: 'entry' + '.css',
+ type: 'asset',
+ source: minifiedCSS,
+ });
+ const fileName = this.getFileName(referenceId);
+
+ // Add CSS file to the page's pageData, so that it will be rendered with
+ // the correct links.
+ getPageDataByViteID(internals, id)?.css.add(fileName);
+ }
+ }
+ }
+
return {
name: PLUGIN_NAME,
@@ -76,7 +130,6 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin {
plugins.splice(viteCSSPostIndex - 1, 0, ourPlugin);
}
},
-
async resolveId(id) {
if (isPageStyleVirtualModule(id)) {
return id;
@@ -108,6 +161,8 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin {
},
async renderChunk(_code, chunk) {
+ if(!legacy) return null;
+
let chunkCSS = '';
let isPureCSS = true;
for (const [id] of Object.entries(chunk.modules)) {
@@ -147,7 +202,7 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin {
},
// Delete CSS chunks so JS is not produced for them.
- generateBundle(opts, bundle) {
+ async generateBundle(opts, bundle) {
const hasPureCSSChunks = internals.pureCSSChunks.size;
const pureChunkFilenames = new Set([...internals.pureCSSChunks].map((chunk) => chunk.fileName));
const emptyChunkFiles = [...pureChunkFilenames]
@@ -156,6 +211,11 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin {
.replace(/\./g, '\\.');
const emptyChunkRE = new RegExp(opts.format === 'es' ? `\\bimport\\s*"[^"]*(?:${emptyChunkFiles})";\n?` : `\\brequire\\(\\s*"[^"]*(?:${emptyChunkFiles})"\\);\n?`, 'g');
+ // Crawl the module graph to find CSS chunks to create
+ if(!legacy) {
+ await addStyles.call(this);
+ }
+
for (const [chunkId, chunk] of Object.entries(bundle)) {
if (chunk.type === 'chunk') {
// This find shared chunks of CSS and adds them to the main CSS chunks,
diff --git a/packages/astro/test/fixtures/page-level-styles/src/layouts/Base.astro b/packages/astro/test/fixtures/page-level-styles/src/layouts/Base.astro
new file mode 100644
index 000000000..f900a1e88
--- /dev/null
+++ b/packages/astro/test/fixtures/page-level-styles/src/layouts/Base.astro
@@ -0,0 +1,13 @@
+---
+
+---
+<html lang="en">
+ <head>
+ <meta charset="utf-8" />
+ <meta name="viewport" content="width=device-width" />
+ <title>Astro</title>
+ </head>
+ <body>
+ <slot />
+ </body>
+</html>
diff --git a/packages/astro/test/fixtures/page-level-styles/src/layouts/Styled.astro b/packages/astro/test/fixtures/page-level-styles/src/layouts/Styled.astro
new file mode 100644
index 000000000..e32ade457
--- /dev/null
+++ b/packages/astro/test/fixtures/page-level-styles/src/layouts/Styled.astro
@@ -0,0 +1,13 @@
+---
+import "../styles/main.css";
+---
+<html lang="en">
+ <head>
+ <meta charset="utf-8" />
+ <meta name="viewport" content="width=device-width" />
+ <title>Astro</title>
+ </head>
+ <body>
+ <slot />
+ </body>
+</html>
diff --git a/packages/astro/test/fixtures/page-level-styles/src/pages/blog.astro b/packages/astro/test/fixtures/page-level-styles/src/pages/blog.astro
new file mode 100644
index 000000000..de9f896a9
--- /dev/null
+++ b/packages/astro/test/fixtures/page-level-styles/src/pages/blog.astro
@@ -0,0 +1,9 @@
+---
+import Layout from '../layouts/Styled.astro'
+---
+
+<Layout>
+ <h1>Astro (with main.css)</h1>
+ <p>This should have custom styles (white text on black background)</p>
+ <a href="/">Home (without main.css)</a>
+</Layout>
diff --git a/packages/astro/test/fixtures/page-level-styles/src/pages/index.astro b/packages/astro/test/fixtures/page-level-styles/src/pages/index.astro
new file mode 100644
index 000000000..d4ae35676
--- /dev/null
+++ b/packages/astro/test/fixtures/page-level-styles/src/pages/index.astro
@@ -0,0 +1,9 @@
+---
+import Layout from '../layouts/Base.astro'
+---
+
+<Layout>
+ <h1>Astro (without main.css)</h1>
+ <p>This should have default styles (white text on black background)</p>
+ <a href="/blog">Blog (with main.css)</a>
+</Layout>
diff --git a/packages/astro/test/fixtures/page-level-styles/src/styles/main.css b/packages/astro/test/fixtures/page-level-styles/src/styles/main.css
new file mode 100644
index 000000000..1bee9c028
--- /dev/null
+++ b/packages/astro/test/fixtures/page-level-styles/src/styles/main.css
@@ -0,0 +1,4 @@
+body {
+ background-color: black;
+ color: white;
+}
diff --git a/packages/astro/test/page-level-styles.test.js b/packages/astro/test/page-level-styles.test.js
new file mode 100644
index 000000000..ae06c6697
--- /dev/null
+++ b/packages/astro/test/page-level-styles.test.js
@@ -0,0 +1,27 @@
+import { expect } from 'chai';
+import { load as cheerioLoad } from 'cheerio';
+import { loadFixture } from './test-utils.js';
+
+// Asset bundling
+describe('Page-level styles', () => {
+ let fixture;
+
+ before(async () => {
+ fixture = await loadFixture({
+ projectRoot: './fixtures/page-level-styles/',
+ });
+ await fixture.build();
+ });
+
+ it('Doesn\'t add page styles for a page without style imports', async () => {
+ let html = await fixture.readFile('/index.html');
+ let $ = await cheerioLoad(html);
+ expect($('link').length).to.equal(0);
+ });
+
+ it('Does add page styles for pages with style imports (or deps)', async () => {
+ let html = await fixture.readFile('/blog/index.html');
+ let $ = await cheerioLoad(html);
+ expect($('link').length).to.equal(1);
+ });
+});