diff --git a/.changeset/kind-squids-warn.md b/.changeset/kind-squids-warn.md new file mode 100644 index 000000000000..31e747cb284f --- /dev/null +++ b/.changeset/kind-squids-warn.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Reintroduces css deduplication for hydrated client components. Ensures assets already added to a client chunk are not flagged as orphaned diff --git a/packages/astro/src/core/build/plugins/plugin-css.ts b/packages/astro/src/core/build/plugins/plugin-css.ts index c8854e408f18..f20540be63ad 100644 --- a/packages/astro/src/core/build/plugins/plugin-css.ts +++ b/packages/astro/src/core/build/plugins/plugin-css.ts @@ -80,7 +80,10 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] { // and that's okay. We can use Rollup's default chunk strategy instead as these CSS // are outside of the SSR build scope, which no dedupe is needed. if (options.target === 'client') { - return internals.cssModuleToChunkIdMap.get(id)!; + // Find the chunkId for this CSS module in the server build. + // If it exists, we can use it to ensure the client build matches the server + // build and doesn't create a duplicate chunk. + return internals.cssModuleToChunkIdMap.get(id); } const ctx = { getModuleInfo: meta.getModuleInfo }; @@ -93,6 +96,7 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] { return chunkId; } } + const chunkId = createNameForParentPages(id, meta); internals.cssModuleToChunkIdMap.set(id, chunkId); return chunkId; @@ -230,8 +234,19 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] { sheetAddedToPage = true; } - if (toBeInlined && sheetAddedToPage) { - // CSS is already added to all used pages, we can delete it from the bundle + const wasInlined = toBeInlined && sheetAddedToPage; + // stylesheets already referenced as an asset by a chunk will not be inlined by + // this plugin, but should not be considered orphaned + const wasAddedToChunk = Object.values(bundle).some((chunk) => + chunk.type === 'chunk' && + chunk.viteMetadata?.importedAssets?.has(id) + ); + const isOrphaned = !sheetAddedToPage && !wasAddedToChunk; + + if (wasInlined || isOrphaned) { + // wasInlined : CSS is already added to all used pages + // isOrphaned : CSS is already used in a merged chunk + // we can delete it from the bundle // and make sure no chunks reference it via `importedCss` (for Vite preloading) // to avoid duplicate CSS. delete bundle[id]; diff --git a/packages/astro/test/css-deduplication.test.js b/packages/astro/test/css-deduplication.test.js new file mode 100644 index 000000000000..829d6790cfc6 --- /dev/null +++ b/packages/astro/test/css-deduplication.test.js @@ -0,0 +1,81 @@ +import * as assert from 'node:assert/strict'; +import { before, describe, it } from 'node:test'; +import * as cheerio from 'cheerio'; +import { loadFixture } from './test-utils.js'; + +describe('CSS deduplication for hydrated components', () => { + describe('inlineStylesheets: never', () => { + let fixture; + before(async () => { + fixture = await loadFixture({ + site: 'https://test.dev/', + root: './fixtures/css-deduplication/', + build: { inlineStylesheets: 'never' }, + outDir: './dist/inline-stylesheets-never', + }); + await fixture.build(); + }); + + it('should not duplicate CSS for hydrated components', async () => { + const assets = await fixture.readdir('/_astro'); + + // Generated file for Counter.css + const COUNTER_CSS_PATH = '/_astro/index.DbgLc3FE.css'; + let file = await fixture.readFile(COUNTER_CSS_PATH); + file = file.replace(/\s+/g, ''); + + for (const fileName of assets) { + const filePath = `/_astro/${fileName}`; + if (filePath === COUNTER_CSS_PATH || !fileName.endsWith('.css')) { + continue; + } + + let r = await fixture.readFile(filePath); + r = r.replace(/\s+/g, ''); + if (file.includes(r)) { + assert.fail(`Duplicate CSS file: ${fileName}`); + } + } + assert.ok(true); + }); + }); + + describe('inlineStylesheets: always', () => { + let fixture; + before(async () => { + fixture = await loadFixture({ + site: 'https://test.dev/', + root: './fixtures/css-deduplication/', + build: { inlineStylesheets: 'always' }, + outDir: './dist/inline-stylesheets-always', + }); + await fixture.build(); + }); + + it('should not emit any .css file when inlineStylesheets is always', async () => { + const assets = await fixture.readdir('/_astro'); + assert.ok(!assets.some((f) => f.endsWith('.css'))); + }); + + it('should not duplicate CSS for hydrated components', async () => { + const html = await fixture.readFile('/index.html'); + const $ = cheerio.load(html); + + // Get all + + +
+ +

Hello, React!

+
+
+ + diff --git a/packages/astro/test/fixtures/css-deduplication/tsconfig.json b/packages/astro/test/fixtures/css-deduplication/tsconfig.json new file mode 100644 index 000000000000..92a18df9052f --- /dev/null +++ b/packages/astro/test/fixtures/css-deduplication/tsconfig.json @@ -0,0 +1,9 @@ +{ + "extends": "astro/tsconfigs/strict", + "include": [".astro/types.d.ts", "**/*"], + "exclude": ["dist"], + "compilerOptions": { + "jsx": "react-jsx", + "jsxImportSource": "react" + } +} diff --git a/packages/astro/test/fixtures/url-import-suffix/astro.config.mjs b/packages/astro/test/fixtures/url-import-suffix/astro.config.mjs new file mode 100644 index 000000000000..e762ba5cf616 --- /dev/null +++ b/packages/astro/test/fixtures/url-import-suffix/astro.config.mjs @@ -0,0 +1,5 @@ +// @ts-check +import { defineConfig } from 'astro/config'; + +// https://astro.build/config +export default defineConfig({}); diff --git a/packages/astro/test/fixtures/url-import-suffix/package.json b/packages/astro/test/fixtures/url-import-suffix/package.json new file mode 100644 index 000000000000..587f4c34aa2a --- /dev/null +++ b/packages/astro/test/fixtures/url-import-suffix/package.json @@ -0,0 +1,8 @@ +{ + "name": "@test/url-import-suffix", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*" + } +} diff --git a/packages/astro/test/fixtures/url-import-suffix/src/assets/index.css b/packages/astro/test/fixtures/url-import-suffix/src/assets/index.css new file mode 100644 index 000000000000..adc68fa6a4df --- /dev/null +++ b/packages/astro/test/fixtures/url-import-suffix/src/assets/index.css @@ -0,0 +1,3 @@ +h1 { + color: red; +} diff --git a/packages/astro/test/fixtures/url-import-suffix/src/assets/style.css b/packages/astro/test/fixtures/url-import-suffix/src/assets/style.css new file mode 100644 index 000000000000..33f920b76136 --- /dev/null +++ b/packages/astro/test/fixtures/url-import-suffix/src/assets/style.css @@ -0,0 +1,3 @@ +h1 { + background: green; +} diff --git a/packages/astro/test/fixtures/url-import-suffix/src/pages/index.astro b/packages/astro/test/fixtures/url-import-suffix/src/pages/index.astro new file mode 100644 index 000000000000..a03e2477c473 --- /dev/null +++ b/packages/astro/test/fixtures/url-import-suffix/src/pages/index.astro @@ -0,0 +1,20 @@ +--- +import assetsUrl from '../assets/index.css?url'; +import assetsUrlNoInline from '../assets/style.css?url&no-inline'; +--- + + + + + + + + Astro + + + + +

Astro

+ + + diff --git a/packages/astro/test/fixtures/url-import-suffix/tsconfig.json b/packages/astro/test/fixtures/url-import-suffix/tsconfig.json new file mode 100644 index 000000000000..38a251b7c1c1 --- /dev/null +++ b/packages/astro/test/fixtures/url-import-suffix/tsconfig.json @@ -0,0 +1,5 @@ +{ + "extends": "astro/tsconfigs/strict", + "include": [".astro/types.d.ts", "**/*"], + "exclude": ["dist"], +} diff --git a/packages/astro/test/url-import-suffix.test.js b/packages/astro/test/url-import-suffix.test.js new file mode 100644 index 000000000000..f5b8b7909668 --- /dev/null +++ b/packages/astro/test/url-import-suffix.test.js @@ -0,0 +1,50 @@ +import * as assert from 'node:assert/strict'; +import { before, describe, it } from 'node:test'; +import * as cheerio from 'cheerio'; +import { loadFixture } from './test-utils.js'; + +describe('imports using ?url suffix', () => { + let fixture; + const assetName = 'index.DqQksVyv.css' + + before(async () => { + fixture = await loadFixture({ root: './fixtures/url-import-suffix/' }); + await fixture.build(); + }); + + it('includes the built asset in the output', async () => { + const assets = await fixture.readdir('/_astro'); + assert.ok(assets.some((f) => f === assetName)); + }); + + it('links the asset in the html', async () => { + const html = await fixture.readFile('/index.html'); + const $ = cheerio.load(html); + + const linkHref = $('link[rel="stylesheet"]').attr('href'); + assert.ok(linkHref, `/_astro/${assetName}`); + }); +}) + +describe('imports using ?url&no-inline suffix', () => { + let fixture; + const assetName = 'style.3WhucSPm.css'; + + before(async () => { + fixture = await loadFixture({ root: './fixtures/url-import-suffix/' }); + await fixture.build(); + }); + + it('includes the built asset in the output', async () => { + const assets = await fixture.readdir('/_astro'); + assert.ok(assets.some((f) => f === assetName)); + }); + + it('links the asset in the html', async () => { + const html = await fixture.readFile('/index.html'); + const $ = cheerio.load(html); + + const linkHref = $('link[rel="stylesheet"]').attr('href'); + assert.ok(linkHref, `/_astro/${assetName}`); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 61b3660777e8..ea986c77c6c1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2925,6 +2925,27 @@ importers: specifier: ^5.39.3 version: 5.40.2 + packages/astro/test/fixtures/css-deduplication: + dependencies: + '@astrojs/react': + specifier: workspace:* + version: link:../../../../integrations/react + '@types/react': + specifier: ^18.3.20 + version: 18.3.26 + '@types/react-dom': + specifier: ^18.3.6 + version: 18.3.7(@types/react@18.3.26) + astro: + specifier: workspace:* + version: link:../../.. + react: + specifier: ^18.3.1 + version: 18.3.1 + react-dom: + specifier: ^18.3.1 + version: 18.3.1(react@18.3.1) + packages/astro/test/fixtures/css-import-as-inline: dependencies: astro: @@ -4397,6 +4418,12 @@ importers: specifier: workspace:* version: link:../../.. + packages/astro/test/fixtures/url-import-suffix: + dependencies: + astro: + specifier: workspace:* + version: link:../../.. + packages/astro/test/fixtures/user-route-priority: dependencies: astro: