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

Remove unused CSS for client:load components #4566

Merged
merged 3 commits into from
Aug 31, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tough-pandas-sneeze.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Remove unused CSS for `client:load` components
13 changes: 6 additions & 7 deletions packages/astro/src/core/build/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ import { prependForwardSlash } from '../path.js';
import { viteID } from '../util.js';

export interface BuildInternals {
// Pure CSS chunks are chunks that only contain CSS.
pureCSSChunks: Set<RenderedChunk>;
Comment on lines -8 to -9
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pureCSSChunks was previously unused. I've re-purposed it as cssChunkModuleIds.

/**
* The module ids of all CSS chunks, used to deduplicate CSS assets between
* SSR build and client build in vite-plugin-css.
*/
cssChunkModuleIds: Set<string>;

// A mapping of hoisted script ids back to the exact hoisted scripts it references
hoistedScriptIdToHoistedMap: Map<string, Set<string>>;
Expand Down Expand Up @@ -59,18 +62,14 @@ export interface BuildInternals {
* @returns {BuildInternals}
*/
export function createBuildInternals(): BuildInternals {
// Pure CSS chunks are chunks that only contain CSS.
// This is all of them, and chunkToReferenceIdMap maps them to a hash id used to find the final file.
const pureCSSChunks = new Set<RenderedChunk>();

// These are for tracking hoisted script bundling
const hoistedScriptIdToHoistedMap = new Map<string, Set<string>>();

// This tracks hoistedScriptId => page components
const hoistedScriptIdToPagesMap = new Map<string, Set<string>>();

return {
pureCSSChunks,
cssChunkModuleIds: new Set(),
hoistedScriptIdToHoistedMap,
hoistedScriptIdToPagesMap,
entrySpecifierToBundleMap: new Map<string, string>(),
Expand Down
23 changes: 21 additions & 2 deletions packages/astro/src/core/build/vite-plugin-css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,30 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]

// Chunks that have the viteMetadata.importedCss are CSS chunks
if (meta.importedCss.size) {
// 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(c.modules)) {
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];
}
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
// client:only component and if so, add its CSS to the page it belongs to.
if (options.target === 'client') {
for (const [id] of Object.entries(c.modules)) {
for (const id of Object.keys(c.modules)) {
for (const pageData of getParentClientOnlys(id, this)) {
for (const importedCssImport of meta.importedCss) {
pageData.css.set(importedCssImport, { depth: -1 });
Expand All @@ -148,7 +167,7 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
}

// For this CSS chunk, walk parents until you find a page. Add the CSS to that page.
for (const [id] of Object.entries(c.modules)) {
for (const id of Object.keys(c.modules)) {
for (const [pageInfo, depth] of walkParentInfos(id, this)) {
if (moduleIsTopLevelPage(pageInfo)) {
const pageViteID = pageInfo.id;
Expand Down
7 changes: 7 additions & 0 deletions packages/astro/test/0-css.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -354,5 +354,12 @@ describe('CSS', function () {
expect(allInjectedStyles).to.contain('.vue-scss{');
expect(allInjectedStyles).to.contain('.vue-scoped[data-v-');
});

it('remove unused styles from client:load components', async () => {
const bundledAssets = await fixture.readdir('./assets');
// SvelteDynamic styles is already included in the main page css asset
const unusedCssAsset = bundledAssets.find((asset) => /SvelteDynamic\..*\.css/.test(asset));
expect(unusedCssAsset, 'Found unused style ' + unusedCssAsset).to.be.undefined;
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<h1 id="svelte-dynamic" class="svelte-dynamic">Svelte Dynamic</h1>

<style>
.svelte-dynamic {
font-family: Courier, monospace;
}
</style>
2 changes: 2 additions & 0 deletions packages/astro/test/fixtures/0-css/src/pages/index.astro
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import VueSass from '../components/VueSass.vue';
import VueScoped from '../components/VueScoped.vue';
import VueScss from '../components/VueScss.vue';
import ReactDynamic from '../components/ReactDynamic.jsx';
import SvelteDynamic from '../components/SvelteDynamic.svelte';

import '../styles/imported-url.css';
import '../styles/imported.sass';
Expand Down Expand Up @@ -69,6 +70,7 @@ import '../styles/imported.scss';
<VueScoped />
<VueScss />
<ReactDynamic client:load />
<SvelteDynamic client:load />
</div>
</body>
</html>