Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CSS deduping and missing chunks #7218

Merged
merged 6 commits into from May 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/eighty-gifts-cheer.md
@@ -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
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
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
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
@@ -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>
@@ -0,0 +1,7 @@
---
import SvelteOnlyAndSsr from './_components/SvelteOnlyAndSsr.svelte'
---

<div>
<SvelteOnlyAndSsr client:only />
</div>
@@ -0,0 +1,7 @@
---
import SvelteOnlyAndSsr from './_components/SvelteOnlyAndSsr.svelte'
---

<div>
<SvelteOnlyAndSsr />
</div>