From 6c7df28ab34b756b8426443bf6976e24d4611a62 Mon Sep 17 00:00:00 2001 From: Bjorn Lu Date: Fri, 26 May 2023 21:30:42 +0800 Subject: [PATCH] Fix CSS deduping and missing chunks (#7218) --- .changeset/eighty-gifts-cheer.md | 5 +++ packages/astro/src/core/build/internal.ts | 11 +++-- .../src/core/build/plugins/plugin-css.ts | 41 ++++++++----------- packages/astro/test/0-css.test.js | 15 +++++++ .../_components/SvelteOnlyAndSsr.svelte | 11 +++++ .../src/pages/client-only-and-ssr/only.astro | 7 ++++ .../src/pages/client-only-and-ssr/ssr.astro | 7 ++++ 7 files changed, 69 insertions(+), 28 deletions(-) create mode 100644 .changeset/eighty-gifts-cheer.md create mode 100644 packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/_components/SvelteOnlyAndSsr.svelte create mode 100644 packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/only.astro create mode 100644 packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/ssr.astro diff --git a/.changeset/eighty-gifts-cheer.md b/.changeset/eighty-gifts-cheer.md new file mode 100644 index 000000000000..41ca0d7f628e --- /dev/null +++ b/.changeset/eighty-gifts-cheer.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fix CSS deduping and missing chunks diff --git a/packages/astro/src/core/build/internal.ts b/packages/astro/src/core/build/internal.ts index 18d28ef74907..d1e2404d6d8a 100644 --- a/packages/astro/src/core/build/internal.ts +++ b/packages/astro/src/core/build/internal.ts @@ -8,10 +8,13 @@ import type { PageBuildData, StylesheetAsset, ViteID } from './types'; export interface BuildInternals { /** - * The module ids of all CSS chunks, used to deduplicate CSS assets between - * SSR build and client build in vite-plugin-css. + * Each CSS module is named with a chunk id derived from the Astro pages they + * are used in by default. It's easy to crawl this relation in the SSR build as + * the Astro pages are the entrypoint, but not for the client build as hydratable + * components are the entrypoint instead. This map is used as a cache from the SSR + * build so the client can pick up the same information and use the same chunk ids. */ - cssChunkModuleIds: Set; + cssModuleToChunkIdMap: Map; // A mapping of hoisted script ids back to the exact hoisted scripts it references hoistedScriptIdToHoistedMap: Map>; @@ -92,7 +95,7 @@ export function createBuildInternals(): BuildInternals { const hoistedScriptIdToPagesMap = new Map>(); return { - cssChunkModuleIds: new Set(), + cssModuleToChunkIdMap: new Map(), hoistedScriptIdToHoistedMap, hoistedScriptIdToPagesMap, entrySpecifierToBundleMap: new Map(), diff --git a/packages/astro/src/core/build/plugins/plugin-css.ts b/packages/astro/src/core/build/plugins/plugin-css.ts index a5bdb70f112b..1cec972239d8 100644 --- a/packages/astro/src/core/build/plugins/plugin-css.ts +++ b/packages/astro/src/core/build/plugins/plugin-css.ts @@ -64,20 +64,7 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] { const cssBuildPlugin: 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 @@ -89,16 +76,31 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] { // For CSS, create a hash of all of the pages that use it. // This causes CSS to be built into shared chunks when used by multiple pages. if (isBuildableCSSRequest(id)) { + // For client builds that has hydrated components as entrypoints, there's no way + // to crawl up and find the pages that use it. So we lookup the cache during SSR + // build (that has the pages information) to derive the same chunk id so they + // match up on build, making sure both builds has the CSS deduped. + // NOTE: Components that are only used with `client:only` may not exist in the cache + // 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)!; + } + for (const [pageInfo] of walkParentInfos(id, { getModuleInfo: meta.getModuleInfo, })) { if (new URL(pageInfo.id, 'file://').searchParams.has(PROPAGATED_ASSET_FLAG)) { // Split delayed assets to separate modules // so they can be injected where needed - return createNameHash(id, [id]); + const chunkId = createNameHash(id, [id]); + internals.cssModuleToChunkIdMap.set(id, chunkId); + return chunkId; } } - return createNameForParentPages(id, meta); + const chunkId = createNameForParentPages(id, meta); + internals.cssModuleToChunkIdMap.set(id, chunkId); + return chunkId; } }, }); @@ -113,15 +115,6 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] { // Skip if the chunk has no CSS, we want to handle CSS chunks only if (meta.importedCss.size < 1) continue; - // In the SSR build, keep track of all CSS chunks' modules as the client build may - // duplicate them, e.g. for `client:load` components that render in SSR and client - // for hydation. - if (options.target === 'server') { - for (const id of Object.keys(chunk.modules)) { - internals.cssChunkModuleIds.add(id); - } - } - // 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 // client:only component and if so, add its CSS to the page it belongs to. diff --git a/packages/astro/test/0-css.test.js b/packages/astro/test/0-css.test.js index 1583891328e4..76bfba296f3c 100644 --- a/packages/astro/test/0-css.test.js +++ b/packages/astro/test/0-css.test.js @@ -265,6 +265,21 @@ describe('CSS', function () { new RegExp(`.svelte-scss.${scopedClass}[^{]*{font-family:ComicSansMS`) ); }); + + it('client:only and SSR in two pages, both should have styles', async () => { + const onlyHtml = await fixture.readFile('/client-only-and-ssr/only/index.html'); + const $onlyHtml = cheerio.load(onlyHtml); + const onlyHtmlCssHref = $onlyHtml('link[rel=stylesheet][href^=/_astro/]').attr('href'); + const onlyHtmlCss = await fixture.readFile(onlyHtmlCssHref.replace(/^\/?/, '/')); + + const ssrHtml = await fixture.readFile('/client-only-and-ssr/ssr/index.html'); + const $ssrHtml = cheerio.load(ssrHtml); + const ssrHtmlCssHref = $ssrHtml('link[rel=stylesheet][href^=/_astro/]').attr('href'); + const ssrHtmlCss = await fixture.readFile(ssrHtmlCssHref.replace(/^\/?/, '/')); + + expect(onlyHtmlCss).to.include('.svelte-only-and-ssr'); + expect(ssrHtmlCss).to.include('.svelte-only-and-ssr'); + }); }); describe('Vite features', () => { diff --git a/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/_components/SvelteOnlyAndSsr.svelte b/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/_components/SvelteOnlyAndSsr.svelte new file mode 100644 index 000000000000..f346f9c04d57 --- /dev/null +++ b/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/_components/SvelteOnlyAndSsr.svelte @@ -0,0 +1,11 @@ + + +
+ Svelte only and SSR +
+ + diff --git a/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/only.astro b/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/only.astro new file mode 100644 index 000000000000..b409693b5d03 --- /dev/null +++ b/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/only.astro @@ -0,0 +1,7 @@ +--- +import SvelteOnlyAndSsr from './_components/SvelteOnlyAndSsr.svelte' +--- + +
+ +
diff --git a/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/ssr.astro b/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/ssr.astro new file mode 100644 index 000000000000..35b7fbae07df --- /dev/null +++ b/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/ssr.astro @@ -0,0 +1,7 @@ +--- +import SvelteOnlyAndSsr from './_components/SvelteOnlyAndSsr.svelte' +--- + +
+ +