Skip to content

Commit

Permalink
Fix CSS deduping and missing chunks (#7218)
Browse files Browse the repository at this point in the history
  • Loading branch information
bluwy authored May 26, 2023
1 parent 6b12f93 commit 6c7df28
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 28 deletions.
5 changes: 5 additions & 0 deletions .changeset/eighty-gifts-cheer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fix CSS deduping and missing chunks
11 changes: 7 additions & 4 deletions packages/astro/src/core/build/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>;
cssModuleToChunkIdMap: Map<string, string>;

// A mapping of hoisted script ids back to the exact hoisted scripts it references
hoistedScriptIdToHoistedMap: Map<string, Set<string>>;
Expand Down Expand Up @@ -92,7 +95,7 @@ export function createBuildInternals(): BuildInternals {
const hoistedScriptIdToPagesMap = new Map<string, Set<string>>();

return {
cssChunkModuleIds: new Set(),
cssModuleToChunkIdMap: new Map(),
hoistedScriptIdToHoistedMap,
hoistedScriptIdToPagesMap,
entrySpecifierToBundleMap: new Map<string, string>(),
Expand Down
41 changes: 17 additions & 24 deletions packages/astro/src/core/build/plugins/plugin-css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
},
});
Expand All @@ -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.
Expand Down
15 changes: 15 additions & 0 deletions packages/astro/test/0-css.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<!-- This file will be used as client:only and SSR on two different pages -->

<div class="svelte-only-and-ssr">
Svelte only and SSR
</div>

<style>
.svelte-only-and-ssr {
background-color: green;
}
</style>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
import SvelteOnlyAndSsr from './_components/SvelteOnlyAndSsr.svelte'
---

<div>
<SvelteOnlyAndSsr client:only />
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
import SvelteOnlyAndSsr from './_components/SvelteOnlyAndSsr.svelte'
---

<div>
<SvelteOnlyAndSsr />
</div>

0 comments on commit 6c7df28

Please sign in to comment.