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
This commit is contained in:
Matthew Phillips 2022-03-28 16:48:06 -04:00 committed by GitHub
parent c40813e932
commit 77354c89bd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 153 additions and 7 deletions

View file

@ -0,0 +1,5 @@
---
'astro': patch
---
Prevent CSS from being added to the wrong pages

View file

@ -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();
}

View file

@ -76,6 +76,7 @@ export async function build(opts: ScanBasedBuildOptions): Promise<RollupOutput |
}),
rollupPluginAstroBuildCSS({
internals,
legacy: true
}),
...(viteConfig.plugins || []),
],

View file

@ -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 || []),
],

View file

@ -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 {

View file

@ -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,

View file

@ -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>

View file

@ -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>

View file

@ -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>

View file

@ -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>

View file

@ -0,0 +1,4 @@
body {
background-color: black;
color: white;
}

View file

@ -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);
});
});