From d83f92a20403ffc8d088cfd13d2806e0f4f1a11e Mon Sep 17 00:00:00 2001 From: Bjorn Lu Date: Thu, 6 Mar 2025 13:47:59 +0800 Subject: [PATCH] Replace internal cssScopeTo implementation to vite.cssScopeTo (#13347) * Replace internal cssScopeTo implementation to vite.cssScopeTo * Fix test * Fix more tests * Again * Again :( * Fix weird name * Add changeset --- .changeset/hungry-sheep-sneeze.md | 5 ++ .../src/core/build/plugins/plugin-css.ts | 53 +------------------ packages/astro/src/core/build/static-build.ts | 13 ++++- packages/astro/src/vite-plugin-astro/index.ts | 22 ++------ packages/astro/src/vite-plugin-astro/types.ts | 21 -------- packages/astro/test/0-css.test.js | 21 +++++--- .../astro/test/astro-css-bundling.test.js | 8 +-- packages/astro/test/config-vite.test.js | 2 +- .../astro/test/css-inline-stylesheets.test.js | 4 +- packages/astro/test/css-order-import.test.js | 4 +- packages/astro/test/css-order.test.js | 2 +- packages/astro/test/postcss.test.js | 14 +++-- packages/astro/test/remote-css.test.js | 12 ++++- .../test/fixtures/basic/astro.config.js | 5 +- 14 files changed, 71 insertions(+), 115 deletions(-) create mode 100644 .changeset/hungry-sheep-sneeze.md diff --git a/.changeset/hungry-sheep-sneeze.md b/.changeset/hungry-sheep-sneeze.md new file mode 100644 index 0000000000..44cf97fd76 --- /dev/null +++ b/.changeset/hungry-sheep-sneeze.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Updates internal CSS chunking behavior for Astro components' scoped styles. This may result in slightly more CSS chunks created, but should allow the scoped styles to only be included on pages that use them. diff --git a/packages/astro/src/core/build/plugins/plugin-css.ts b/packages/astro/src/core/build/plugins/plugin-css.ts index c39d4da6f0..d985a6a531 100644 --- a/packages/astro/src/core/build/plugins/plugin-css.ts +++ b/packages/astro/src/core/build/plugins/plugin-css.ts @@ -1,12 +1,11 @@ import type { GetModuleInfo } from 'rollup'; -import type { BuildOptions, ResolvedConfig, Rollup, Plugin as VitePlugin } from 'vite'; +import type { BuildOptions, ResolvedConfig, Plugin as VitePlugin } from 'vite'; import { isBuildableCSSRequest } from '../../../vite-plugin-astro-server/util.js'; import type { BuildInternals } from '../internal.js'; import type { AstroBuildPlugin, BuildTarget } from '../plugin.js'; import type { PageBuildData, StaticBuildOptions, StylesheetAsset } from '../types.js'; import { hasAssetPropagationFlag } from '../../../content/index.js'; -import type { AstroPluginCssMetadata } from '../../../vite-plugin-astro/index.js'; import * as assetName from '../css-asset-name.js'; import { getParentExtendedModuleInfos, @@ -156,32 +155,6 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] { }, }; - /** - * This plugin is a port of https://github.com/vitejs/vite/pull/16058. It enables removing unused - * scoped CSS from the bundle if the scoped target (e.g. Astro files) were not bundled. - * Once/If that PR is merged, we can refactor this away, renaming `meta.astroCss` to `meta.vite`. - */ - const cssScopeToPlugin: VitePlugin = { - name: 'astro:rollup-plugin-css-scope-to', - renderChunk(_, chunk, __, meta) { - for (const id in chunk.modules) { - // If this CSS is scoped to its importers exports, check if those importers exports - // are rendered in the chunks. If they are not, we can skip bundling this CSS. - const modMeta = this.getModuleInfo(id)?.meta as AstroPluginCssMetadata | undefined; - const cssScopeTo = modMeta?.astroCss?.cssScopeTo; - if (cssScopeTo && !isCssScopeToRendered(cssScopeTo, Object.values(meta.chunks))) { - // If this CSS is not used, delete it from the chunk modules so that Vite is unable - // to trace that it's used - delete chunk.modules[id]; - const moduleIdsIndex = chunk.moduleIds.indexOf(id); - if (moduleIdsIndex > -1) { - chunk.moduleIds.splice(moduleIdsIndex, 1); - } - } - } - }, - }; - const singleCssPlugin: VitePlugin = { name: 'astro:rollup-plugin-single-css', enforce: 'post', @@ -273,7 +246,7 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] { }, }; - return [cssBuildPlugin, cssScopeToPlugin, singleCssPlugin, inlineStylesheetsPlugin]; + return [cssBuildPlugin, singleCssPlugin, inlineStylesheetsPlugin]; } /***** UTILITY FUNCTIONS *****/ @@ -321,25 +294,3 @@ function appendCSSToPage( } } } - -/** - * `cssScopeTo` is a map of `importer`s to its `export`s. This function iterate each `cssScopeTo` entries - * and check if the `importer` and its `export`s exists in the final chunks. If at least one matches, - * `cssScopeTo` is considered "rendered" by Rollup and we return true. - */ -function isCssScopeToRendered( - cssScopeTo: Record, - chunks: Rollup.RenderedChunk[], -) { - for (const moduleId in cssScopeTo) { - const exports = cssScopeTo[moduleId]; - // Find the chunk that renders this `moduleId` and get the rendered module - const renderedModule = chunks.find((c) => c.moduleIds.includes(moduleId))?.modules[moduleId]; - // Return true if `renderedModule` exists and one of its exports is rendered - if (renderedModule?.renderedExports.some((e) => exports.includes(e))) { - return true; - } - } - - return false; -} diff --git a/packages/astro/src/core/build/static-build.ts b/packages/astro/src/core/build/static-build.ts index 9e903696c6..eeecacf33a 100644 --- a/packages/astro/src/core/build/static-build.ts +++ b/packages/astro/src/core/build/static-build.ts @@ -188,7 +188,18 @@ async function ssrBuild( const encoded = encodeName(name); return [prefix, encoded, suffix].join(''); }, - assetFileNames: `${settings.config.build.assets}/[name].[hash][extname]`, + assetFileNames(chunkInfo) { + const { names } = chunkInfo; + const name = names[0] ?? ''; + + // Sometimes chunks have the `@_@astro` suffix due to SSR logic. Remove it! + // TODO: refactor our build logic to avoid this + if (name.includes(ASTRO_PAGE_EXTENSION_POST_PATTERN)) { + const [sanitizedName] = name.split(ASTRO_PAGE_EXTENSION_POST_PATTERN); + return `${settings.config.build.assets}/${sanitizedName}.[hash][extname]`; + } + return `${settings.config.build.assets}/[name].[hash][extname]`; + }, ...viteConfig.build?.rollupOptions?.output, entryFileNames(chunkInfo) { if (chunkInfo.facadeModuleId?.startsWith(ASTRO_PAGE_RESOLVED_MODULE_ID)) { diff --git a/packages/astro/src/vite-plugin-astro/index.ts b/packages/astro/src/vite-plugin-astro/index.ts index 21d9dcfb14..c86286e2de 100644 --- a/packages/astro/src/vite-plugin-astro/index.ts +++ b/packages/astro/src/vite-plugin-astro/index.ts @@ -2,11 +2,7 @@ import type { SourceDescription } from 'rollup'; import type * as vite from 'vite'; import type { Logger } from '../core/logger/core.js'; import type { AstroSettings } from '../types/astro.js'; -import type { - PluginCssMetadata as AstroPluginCssMetadata, - PluginMetadata as AstroPluginMetadata, - CompileMetadata, -} from './types.js'; +import type { PluginMetadata as AstroPluginMetadata, CompileMetadata } from './types.js'; import { defaultClientConditions, defaultServerConditions, normalizePath } from 'vite'; import type { AstroConfig } from '../types/public/config.js'; @@ -16,7 +12,7 @@ import { handleHotUpdate } from './hmr.js'; import { parseAstroRequest } from './query.js'; import { loadId } from './utils.js'; export { getAstroMetadata } from './metadata.js'; -export type { AstroPluginMetadata, AstroPluginCssMetadata }; +export type { AstroPluginMetadata }; interface AstroPluginOptions { settings: AstroSettings; @@ -138,17 +134,9 @@ export default function astro({ settings, logger }: AstroPluginOptions): vite.Pl return { code: result.code, - // This metadata is used by `cssScopeToPlugin` to remove this module from the bundle - // if the `filename` default export (the Astro component) is unused. - meta: result.isGlobal - ? undefined - : ({ - astroCss: { - cssScopeTo: { - [filename]: ['default'], - }, - }, - } satisfies AstroPluginCssMetadata), + // `vite.cssScopeTo` is a Vite feature that allows this CSS to be treeshaken + // if the Astro component's default export is not used + meta: result.isGlobal ? undefined : { vite: { cssScopeTo: [filename, 'default'] } }, }; } case 'script': { diff --git a/packages/astro/src/vite-plugin-astro/types.ts b/packages/astro/src/vite-plugin-astro/types.ts index d85fd64830..61cd6db44d 100644 --- a/packages/astro/src/vite-plugin-astro/types.ts +++ b/packages/astro/src/vite-plugin-astro/types.ts @@ -18,27 +18,6 @@ export interface PluginMetadata { }; } -export interface PluginCssMetadata { - astroCss: { - /** - * For Astro CSS virtual modules, it can scope to the main Astro module's default export - * so that if those exports are treeshaken away, the CSS module will also be treeshaken. - * - * Example config if the CSS id is `/src/Foo.astro?astro&type=style&lang.css`: - * ```js - * cssScopeTo: { - * '/src/Foo.astro': ['default'] - * } - * ``` - * - * The above is the only config we use today, but we're exposing as a `Record` to follow the - * upstream Vite implementation: https://github.com/vitejs/vite/pull/16058. When/If that lands, - * we can also remove our custom implementation. - */ - cssScopeTo: Record; - }; -} - export interface CompileMetadata { /** Used for HMR to compare code changes */ originalCode: string; diff --git a/packages/astro/test/0-css.test.js b/packages/astro/test/0-css.test.js index 65010f5803..e200c7d562 100644 --- a/packages/astro/test/0-css.test.js +++ b/packages/astro/test/0-css.test.js @@ -9,10 +9,19 @@ import { after, before, describe, it } from 'node:test'; import * as cheerio from 'cheerio'; import { loadFixture } from './test-utils.js'; -/** @type {import('./test-utils').Fixture} */ -let fixture; +async function getCssContent($, fixture) { + const contents = await Promise.all( + $('link[rel=stylesheet][href^=/_astro/]').map((_, el) => + fixture.readFile(el.attribs.href.replace(/^\/?/, '/')), + ), + ); + return contents.join('').replace(/\s/g, '').replace('/n', ''); +} describe('CSS', function () { + /** @type {import('./test-utils').Fixture} */ + let fixture; + before(async () => { fixture = await loadFixture({ root: './fixtures/0-css/' }); }); @@ -30,10 +39,7 @@ describe('CSS', function () { // get bundled CSS (will be hashed, hence DOM query) html = await fixture.readFile('/index.html'); $ = cheerio.load(html); - const bundledCSSHREF = $('link[rel=stylesheet][href^=/_astro/]').attr('href'); - bundledCSS = (await fixture.readFile(bundledCSSHREF.replace(/^\/?/, '/'))) - .replace(/\s/g, '') - .replace('/n', ''); + bundledCSS = await getCssContent($, fixture); }, { timeout: 45000, @@ -99,8 +105,7 @@ describe('CSS', function () { it('Styles through barrel files should only include used Astro scoped styles', async () => { const barrelHtml = await fixture.readFile('/barrel-styles/index.html'); const barrel$ = cheerio.load(barrelHtml); - const barrelBundledCssHref = barrel$('link[rel=stylesheet][href^=/_astro/]').attr('href'); - const style = await fixture.readFile(barrelBundledCssHref.replace(/^\/?/, '/')); + const style = await getCssContent(barrel$, fixture); assert.match(style, /\.comp-a\[data-astro-cid/); assert.match(style, /\.comp-c\{/); assert.doesNotMatch(style, /\.comp-b/); diff --git a/packages/astro/test/astro-css-bundling.test.js b/packages/astro/test/astro-css-bundling.test.js index 92aa6db802..8319563ff5 100644 --- a/packages/astro/test/astro-css-bundling.test.js +++ b/packages/astro/test/astro-css-bundling.test.js @@ -63,9 +63,9 @@ describe('CSS Bundling', function () { } }); - it('there are 4 css files', async () => { + it('there are 5 css files', async () => { const dir = await fixture.readdir('/_astro'); - assert.equal(dir.length, 4); + assert.equal(dir.length, 5); }); it('CSS includes hashes', async () => { @@ -96,9 +96,9 @@ describe('CSS Bundling', function () { await fixture.build({ mode: 'production' }); }); - it('there are 4 css files', async () => { + it('there are 5 css files', async () => { const dir = await fixture.readdir('/assets'); - assert.equal(dir.length, 4); + assert.equal(dir.length, 5); }); it('CSS does not include hashes', async () => { diff --git a/packages/astro/test/config-vite.test.js b/packages/astro/test/config-vite.test.js index 90d3487f9f..8d9bb74f5c 100644 --- a/packages/astro/test/config-vite.test.js +++ b/packages/astro/test/config-vite.test.js @@ -20,7 +20,7 @@ describe('Vite Config', async () => { it('Allows overriding bundle naming options in the build', async () => { const html = await fixture.readFile('/index.html'); const $ = cheerio.load(html); - assert.match($('link').attr('href'), /\/assets\/testing-[a-z\d]+\.css/); + assert.match($('link').attr('href'), /\/assets\/testing-.+\.css/); }); }); diff --git a/packages/astro/test/css-inline-stylesheets.test.js b/packages/astro/test/css-inline-stylesheets.test.js index 7bae334e14..53771c99a0 100644 --- a/packages/astro/test/css-inline-stylesheets.test.js +++ b/packages/astro/test/css-inline-stylesheets.test.js @@ -113,7 +113,7 @@ describe('Setting inlineStylesheets to auto in static output', () => { // the count of style/link tags depends on our css chunking logic // this test should be updated if it changes - assert.equal($('style').length, 3); + assert.equal($('style').length, 2); assert.equal($('link[rel=stylesheet]').length, 1); }); @@ -162,7 +162,7 @@ describe('Setting inlineStylesheets to auto in server output', () => { // the count of style/link tags depends on our css chunking logic // this test should be updated if it changes - assert.equal($('style').length, 3); + assert.equal($('style').length, 2); assert.equal($('link[rel=stylesheet]').length, 1); }); diff --git a/packages/astro/test/css-order-import.test.js b/packages/astro/test/css-order-import.test.js index f8a9cf5b2b..9b67f1580c 100644 --- a/packages/astro/test/css-order-import.test.js +++ b/packages/astro/test/css-order-import.test.js @@ -147,9 +147,9 @@ describe('CSS ordering - import order', () => { const content = await Promise.all( getLinks(html).map((href) => getLinkContent(href, fixture)), ); - let [link1, link2] = content; + let [link1, , link3] = content; assert.ok(link1.css.includes('f0f8ff')); // aliceblue minified - assert.ok(link2.css.includes('ff0')); // yellow minified + assert.ok(link3.css.includes('ff0')); // yellow minified }); }); }); diff --git a/packages/astro/test/css-order.test.js b/packages/astro/test/css-order.test.js index 46bacd0f8c..70fd44c392 100644 --- a/packages/astro/test/css-order.test.js +++ b/packages/astro/test/css-order.test.js @@ -90,7 +90,7 @@ describe('CSS production ordering', () => { ); assert.ok(content.length, 3, 'there are 3 stylesheets'); - const [, sharedStyles, pageStyles] = content; + const [, pageStyles, sharedStyles] = content; assert.ok(/red/.exec(sharedStyles.css)); assert.ok(/#00f/.exec(pageStyles.css)); diff --git a/packages/astro/test/postcss.test.js b/packages/astro/test/postcss.test.js index 145db478f4..35cb42c383 100644 --- a/packages/astro/test/postcss.test.js +++ b/packages/astro/test/postcss.test.js @@ -4,6 +4,15 @@ import * as cheerio from 'cheerio'; import eol from 'eol'; import { loadFixture } from './test-utils.js'; +async function getCssContent($, fixture) { + const contents = await Promise.all( + $('link[rel=stylesheet][href^=/_astro/]').map((_, el) => + fixture.readFile(el.attribs.href.replace(/^\/?/, '/')), + ), + ); + return contents.join('').replace(/\s/g, '').replace('/n', ''); +} + describe('PostCSS', () => { let fixture; let bundledCSS; @@ -19,10 +28,7 @@ describe('PostCSS', () => { // get bundled CSS (will be hashed, hence DOM query) const html = await fixture.readFile('/index.html'); const $ = cheerio.load(html); - const bundledCSSHREF = $('link[rel=stylesheet][href^=/_astro/]').attr('href'); - bundledCSS = (await fixture.readFile(bundledCSSHREF.replace(/^\/?/, '/'))) - .replace(/\s/g, '') - .replace('/n', ''); + bundledCSS = await getCssContent($, fixture); }, { timeout: 45000 }, ); diff --git a/packages/astro/test/remote-css.test.js b/packages/astro/test/remote-css.test.js index b6b0b03a1a..af87068887 100644 --- a/packages/astro/test/remote-css.test.js +++ b/packages/astro/test/remote-css.test.js @@ -3,6 +3,15 @@ import { before, describe, it } from 'node:test'; import * as cheerio from 'cheerio'; import { loadFixture } from './test-utils.js'; +async function getCssContent($, fixture) { + const contents = await Promise.all( + $('link[rel=stylesheet][href^=/_astro/]').map((_, el) => + fixture.readFile(el.attribs.href.replace(/^\/?/, '/')), + ), + ); + return contents.join('').replace(/\s/g, '').replace('/n', ''); +} + describe('Remote CSS', () => { let fixture; @@ -19,8 +28,7 @@ describe('Remote CSS', () => { const html = await fixture.readFile('/index.html'); const $ = cheerio.load(html); - const relPath = $('link').attr('href'); - const css = await fixture.readFile(relPath); + const css = await getCssContent($, fixture); assert.match(css, /https:\/\/unpkg.com\/open-props/); assert.match(css, /body/); diff --git a/packages/integrations/tailwind/test/fixtures/basic/astro.config.js b/packages/integrations/tailwind/test/fixtures/basic/astro.config.js index 502f5ca17e..98f6925aef 100644 --- a/packages/integrations/tailwind/test/fixtures/basic/astro.config.js +++ b/packages/integrations/tailwind/test/fixtures/basic/astro.config.js @@ -8,5 +8,8 @@ export default defineConfig({ configFile: fileURLToPath(new URL('./tailwind.config.js', import.meta.url)), nesting: true }), - ] + ], + build: { + inlineStylesheets: 'never' + } });