diff options
author | 2022-03-28 16:48:06 -0400 | |
---|---|---|
committer | 2022-03-28 16:48:06 -0400 | |
commit | 77354c89bd606beba71231cce6ce935905de68a7 (patch) | |
tree | 58fe284c035d8deb33da0ffa2c17138b4d954854 | |
parent | c40813e932c1589f37ee6d63e2d495679c4bedb0 (diff) | |
download | astro-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
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); + }); +}); |