diff --git a/.changeset/violet-islands-buy.md b/.changeset/violet-islands-buy.md new file mode 100644 index 000000000000..8c88d23be86c --- /dev/null +++ b/.changeset/violet-islands-buy.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fix CSS chunking and deduping between multiple Astro files and framework components diff --git a/packages/astro/src/core/build/plugins/plugin-css.ts b/packages/astro/src/core/build/plugins/plugin-css.ts index 7a59004607d7..2df1114879de 100644 --- a/packages/astro/src/core/build/plugins/plugin-css.ts +++ b/packages/astro/src/core/build/plugins/plugin-css.ts @@ -55,7 +55,20 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] { name: 'astro:rollup-plugin-build-css', + transform(_, id) { + // In the SSR build, styles that are bundled are tracked in `internals.cssChunkModuleIds`. + // In the client build, if we're also bundling the same style, return an empty string to + // deduplicate the final CSS output. + if (options.target === 'client' && internals.cssChunkModuleIds.has(id)) { + return ''; + } + }, + outputOptions(outputOptions) { + // Skip in client builds as its module graph doesn't have reference to Astro pages + // to be able to chunk based on it's related top-level pages. + if (options.target === 'client') return; + const assetFileNames = outputOptions.assetFileNames; const namingIncludesHash = assetFileNames?.toString().includes('[hash]'); const createNameForParentPages = namingIncludesHash @@ -133,17 +146,6 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] internals.cssChunkModuleIds.add(id); } } - // In the client build, we bail if the chunk is a duplicated CSS chunk tracked from - // above. We remove all the importedCss to prevent emitting the CSS asset. - if (options.target === 'client') { - if (Object.keys(c.modules).every((id) => internals.cssChunkModuleIds.has(id))) { - for (const importedCssImport of meta.importedCss) { - delete bundle[importedCssImport]; - meta.importedCss.delete(importedCssImport); - } - return; - } - } // For the client build, client:only styles need to be mapped // over to their page. For this chunk, determine if it's a child of a diff --git a/packages/astro/test/astro-client-only.test.js b/packages/astro/test/astro-client-only.test.js index f3329c683bcf..c003e0a28622 100644 --- a/packages/astro/test/astro-client-only.test.js +++ b/packages/astro/test/astro-client-only.test.js @@ -28,8 +28,12 @@ describe('Client only components', () => { const html = await fixture.readFile('/index.html'); const $ = cheerioLoad(html); - const href = $('link[rel=stylesheet]').attr('href'); - const css = await fixture.readFile(href); + const stylesheets = await Promise.all( + $('link[rel=stylesheet]').map((_, el) => { + return fixture.readFile(el.attribs.href); + }) + ); + const css = stylesheets.join(''); expect(css).to.match(/yellowgreen/, 'Svelte styles are added'); expect(css).to.match(/Courier New/, 'Global styles are added'); @@ -87,8 +91,12 @@ describe('Client only components subpath', () => { const html = await fixture.readFile('/index.html'); const $ = cheerioLoad(html); - const href = $('link[rel=stylesheet]').attr('href'); - const css = await fixture.readFile(href.replace(/\/blog/, '')); + const stylesheets = await Promise.all( + $('link[rel=stylesheet]').map((_, el) => { + return fixture.readFile(el.attribs.href.replace(/\/blog/, '')); + }) + ); + const css = stylesheets.join(''); expect(css).to.match(/yellowgreen/, 'Svelte styles are added'); expect(css).to.match(/Courier New/, 'Global styles are added'); diff --git a/packages/astro/test/css-order-import.test.js b/packages/astro/test/css-order-import.test.js index 01b6f5497199..745187c539c7 100644 --- a/packages/astro/test/css-order-import.test.js +++ b/packages/astro/test/css-order-import.test.js @@ -106,6 +106,25 @@ describe('CSS ordering - import order', () => { expect(idx1).to.be.greaterThan(idx2); expect(idx2).to.be.greaterThan(idx3); }); + + it('correctly chunks css import from framework components', async () => { + let html = await fixture.readFile('/index.html'); + + const content = await Promise.all(getLinks(html).map((href) => getLinkContent(href))); + const [, { css }] = content; + expect(css).to.not.include( + '.client-1{background:red!important}', + 'CSS from Client2.jsx leaked into index.astro when chunking' + ); + }); + + it('dedupe css between astro and framework components', async () => { + let html = await fixture.readFile('/dedupe/index.html'); + + const content = await Promise.all(getLinks(html).map((href) => getLinkContent(href))); + const css = content.map((c) => c.css).join(''); + expect(css.match(/\.astro-jsx/)?.length).to.eq(1, '.astro-jsx class is duplicated'); + }); }); describe('Dynamic import', () => { diff --git a/packages/astro/test/fixtures/css-order-import/astro.config.mjs b/packages/astro/test/fixtures/css-order-import/astro.config.mjs new file mode 100644 index 000000000000..1c9fe511ffce --- /dev/null +++ b/packages/astro/test/fixtures/css-order-import/astro.config.mjs @@ -0,0 +1,7 @@ +import { defineConfig } from 'astro/config'; +import react from '@astrojs/react' + +// https://astro.build/config +export default defineConfig({ + integrations: [react()] +}); diff --git a/packages/astro/test/fixtures/css-order-import/package.json b/packages/astro/test/fixtures/css-order-import/package.json index 2901a838fa3b..caf2346ea5d0 100644 --- a/packages/astro/test/fixtures/css-order-import/package.json +++ b/packages/astro/test/fixtures/css-order-import/package.json @@ -1,6 +1,9 @@ { "name": "@test/css-order-import", "dependencies": { - "astro": "workspace:*" + "@astrojs/react": "workspace:*", + "astro": "workspace:*", + "react": "^18.1.0", + "react-dom": "^18.1.0" } } diff --git a/packages/astro/test/fixtures/css-order-import/src/components/Client1.jsx b/packages/astro/test/fixtures/css-order-import/src/components/Client1.jsx new file mode 100644 index 000000000000..2cec90a9a67f --- /dev/null +++ b/packages/astro/test/fixtures/css-order-import/src/components/Client1.jsx @@ -0,0 +1,5 @@ +import "../styles/Client1.css" + +export default function Client() { + return
Client 1
; +} diff --git a/packages/astro/test/fixtures/css-order-import/src/components/Client2.jsx b/packages/astro/test/fixtures/css-order-import/src/components/Client2.jsx new file mode 100644 index 000000000000..226f546792e7 --- /dev/null +++ b/packages/astro/test/fixtures/css-order-import/src/components/Client2.jsx @@ -0,0 +1,5 @@ +import "../styles/Client2.css" + +export default function Client() { + return
Client 2
; +} diff --git a/packages/astro/test/fixtures/css-order-import/src/components/Dedupe.jsx b/packages/astro/test/fixtures/css-order-import/src/components/Dedupe.jsx new file mode 100644 index 000000000000..8b8889251b3d --- /dev/null +++ b/packages/astro/test/fixtures/css-order-import/src/components/Dedupe.jsx @@ -0,0 +1,5 @@ +import '../styles/AstroJsx.css'; + +export default function Dedupe() { + return
Dedupe Astro JSX
; +} diff --git a/packages/astro/test/fixtures/css-order-import/src/pages/component.astro b/packages/astro/test/fixtures/css-order-import/src/pages/component.astro index 018ab1866f5d..1b779d18db03 100644 --- a/packages/astro/test/fixtures/css-order-import/src/pages/component.astro +++ b/packages/astro/test/fixtures/css-order-import/src/pages/component.astro @@ -1,6 +1,7 @@ --- import One from '../components/One.astro'; import Two from '../components/Two.astro'; +import Client2 from '../components/Client2.jsx'; --- @@ -15,5 +16,6 @@ import Two from '../components/Two.astro';

Test

+ diff --git a/packages/astro/test/fixtures/css-order-import/src/pages/dedupe.astro b/packages/astro/test/fixtures/css-order-import/src/pages/dedupe.astro new file mode 100644 index 000000000000..ea3a01670665 --- /dev/null +++ b/packages/astro/test/fixtures/css-order-import/src/pages/dedupe.astro @@ -0,0 +1,13 @@ +--- +import '../styles/AstroJsx.css'; +import Dedupe from '../components/Dedupe.jsx'; +--- + + + Test + + +

Test

+ + + diff --git a/packages/astro/test/fixtures/css-order-import/src/pages/index.astro b/packages/astro/test/fixtures/css-order-import/src/pages/index.astro index 0843250c04f6..0a6baab59486 100644 --- a/packages/astro/test/fixtures/css-order-import/src/pages/index.astro +++ b/packages/astro/test/fixtures/css-order-import/src/pages/index.astro @@ -1,5 +1,6 @@ --- import '../styles/base.css'; +import Client1 from '../components/Client1.jsx'; --- @@ -12,5 +13,6 @@ import '../styles/base.css';

Test

+ diff --git a/packages/astro/test/fixtures/css-order-import/src/styles/AstroJsx.css b/packages/astro/test/fixtures/css-order-import/src/styles/AstroJsx.css new file mode 100644 index 000000000000..c69af56084ed --- /dev/null +++ b/packages/astro/test/fixtures/css-order-import/src/styles/AstroJsx.css @@ -0,0 +1,4 @@ +/* this css is imported in both .astro and .jsx component, make sure CSS is deduped */ +.astro-jsx { + color: red; +} diff --git a/packages/astro/test/fixtures/css-order-import/src/styles/Client1.css b/packages/astro/test/fixtures/css-order-import/src/styles/Client1.css new file mode 100644 index 000000000000..86eb7de5d14b --- /dev/null +++ b/packages/astro/test/fixtures/css-order-import/src/styles/Client1.css @@ -0,0 +1,3 @@ +.client-1 { + background: green; +} diff --git a/packages/astro/test/fixtures/css-order-import/src/styles/Client2.css b/packages/astro/test/fixtures/css-order-import/src/styles/Client2.css new file mode 100644 index 000000000000..10acac0f300f --- /dev/null +++ b/packages/astro/test/fixtures/css-order-import/src/styles/Client2.css @@ -0,0 +1,10 @@ +/* + This cheeky css tries to affect index.astro, the good thing is that the Client2.jsx component is + only loaded in component.astro. So index.astro's Client1.jsx component should not be affected by this. +*/ +.client-1 { + background: red !important; +} +.client-2 { + background: green; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2d961d786238..d623f0514b6b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1949,9 +1949,15 @@ importers: packages/astro/test/fixtures/css-order-import: specifiers: + '@astrojs/react': workspace:* astro: workspace:* + react: ^18.1.0 + react-dom: ^18.1.0 dependencies: + '@astrojs/react': link:../../../../integrations/react astro: link:../../.. + react: 18.2.0 + react-dom: 18.2.0_react@18.2.0 packages/astro/test/fixtures/css-order-layout: specifiers: