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

Treeshake unused Astro scoped styles #10291

Merged
merged 4 commits into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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/nine-trains-drop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Treeshakes unused Astro component scoped styles
48 changes: 46 additions & 2 deletions packages/astro/src/core/build/plugins/plugin-css.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import type { GetModuleInfo } from 'rollup';
import type { BuildOptions, Plugin as VitePlugin, ResolvedConfig } from 'vite';
import type { BuildOptions, Plugin as VitePlugin, ResolvedConfig, Rollup } from 'vite';
import { isBuildableCSSRequest } from '../../../vite-plugin-astro-server/util.js';
import type { BuildInternals } from '../internal.js';
import type { AstroBuildPlugin, BuildTarget } from '../plugin.js';
import type { PageBuildData, StaticBuildOptions, StylesheetAsset } from '../types.js';

import { PROPAGATED_ASSET_FLAG } from '../../../content/consts.js';
import type { AstroPluginCssMetadata } from '../../../vite-plugin-astro/index.js';
import * as assetName from '../css-asset-name.js';
import { moduleIsTopLevelPage, walkParentInfos } from '../graph.js';
import {
Expand Down Expand Up @@ -180,6 +181,32 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {
},
};

/**
* This plugin is a port of https://github.com/vitejs/vite/pull/16058. It enables removing unused
* scoped CSS from the bundle if the scoped target (e.g. Astro files) were not bundled.
* Once/If that PR is merged, we can refactor this away, renaming `meta.astroCss` to `meta.vite`.
*/
const cssScopeToPlugin: VitePlugin = {
name: 'astro:rollup-plugin-css-scope-to',
renderChunk(_, chunk, __, meta) {
for (const id in chunk.modules) {
// If this CSS is scoped to its importers exports, check if those importers exports
// are rendered in the chunks. If they are not, we can skip bundling this CSS.
const modMeta = this.getModuleInfo(id)?.meta as AstroPluginCssMetadata | undefined;
const cssScopeTo = modMeta?.astroCss?.cssScopeTo;
if (cssScopeTo && !isCssScopeToRendered(cssScopeTo, Object.values(meta.chunks))) {
// If this CSS is not used, delete it from the chunk modules so that Vite is unable
// to trace that it's used
delete chunk.modules[id];
const moduleIdsIndex = chunk.moduleIds.indexOf(id);
if (moduleIdsIndex > -1) {
chunk.moduleIds.splice(moduleIdsIndex, 1);
}
}
}
},
};

const singleCssPlugin: VitePlugin = {
name: 'astro:rollup-plugin-single-css',
enforce: 'post',
Expand Down Expand Up @@ -283,7 +310,7 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {
},
};

return [cssBuildPlugin, singleCssPlugin, inlineStylesheetsPlugin];
return [cssBuildPlugin, cssScopeToPlugin, singleCssPlugin, inlineStylesheetsPlugin];
}

/***** UTILITY FUNCTIONS *****/
Expand Down Expand Up @@ -331,3 +358,20 @@ function appendCSSToPage(
}
}
}

function isCssScopeToRendered(
bluwy marked this conversation as resolved.
Show resolved Hide resolved
cssScopeTo: Record<string, string[]>,
chunks: Rollup.RenderedChunk[]
) {
for (const moduleId in cssScopeTo) {
const exports = cssScopeTo[moduleId];
// Find the chunk that renders this `moduleId` and get the rendered module
const renderedModule = chunks.find((c) => c.moduleIds.includes(moduleId))?.modules[moduleId];

if (renderedModule?.renderedExports.some((e) => exports.includes(e))) {
return true;
}
}

return false;
}
17 changes: 5 additions & 12 deletions packages/astro/src/core/compile/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import type { AstroError } from '../errors/errors.js';
import { AggregateError, CompilerError } from '../errors/errors.js';
import { AstroErrorData } from '../errors/index.js';
import { resolvePath } from '../util.js';
import { createStylePreprocessor } from './style.js';
import { type PartialCompileCssResult, createStylePreprocessor } from './style.js';
import type { CompileCssResult } from './types.js';

export interface CompileProps {
astroConfig: AstroConfig;
Expand All @@ -20,14 +21,6 @@ export interface CompileProps {
source: string;
}

export interface CompileCssResult {
code: string;
/**
* The dependencies of the transformed CSS (Normalized paths)
*/
dependencies?: string[];
}

export interface CompileResult extends Omit<TransformResult, 'css'> {
css: CompileCssResult[];
}
Expand All @@ -42,7 +35,7 @@ export async function compile({
// Because `@astrojs/compiler` can't return the dependencies for each style transformed,
// we need to use an external array to track the dependencies whenever preprocessing is called,
// and we'll rebuild the final `css` result after transformation.
const cssDeps: CompileCssResult['dependencies'][] = [];
const cssPartialCompileResults: PartialCompileCssResult[] = [];
Comment on lines -45 to +38
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: I refactored this from string[][] to { ... dependencies: string[] }[] because I needed to keep an extra isGlobal metadata whenever preprocessing is called.

const cssTransformErrors: AstroError[] = [];
let transformResult: TransformResult;

Expand All @@ -68,7 +61,7 @@ export async function compile({
preprocessStyle: createStylePreprocessor({
filename,
viteConfig,
cssDeps,
cssPartialCompileResults,
cssTransformErrors,
}),
async resolvePath(specifier) {
Expand All @@ -93,8 +86,8 @@ export async function compile({
return {
...transformResult,
css: transformResult.css.map((code, i) => ({
...cssPartialCompileResults[i],
code,
dependencies: cssDeps[i],
})),
};
}
Expand Down
15 changes: 9 additions & 6 deletions packages/astro/src/core/compile/style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,19 @@ import fs from 'node:fs';
import type { TransformOptions } from '@astrojs/compiler';
import { type ResolvedConfig, normalizePath, preprocessCSS } from 'vite';
import { AstroErrorData, CSSError, positionAt } from '../errors/index.js';
import type { CompileCssResult } from './compile.js';
import type { CompileCssResult } from './types.js';

export type PartialCompileCssResult = Pick<CompileCssResult, 'isGlobal' | 'dependencies'>;

export function createStylePreprocessor({
filename,
viteConfig,
cssDeps,
cssPartialCompileResults,
cssTransformErrors,
}: {
filename: string;
viteConfig: ResolvedConfig;
cssDeps: CompileCssResult['dependencies'][];
cssPartialCompileResults: Partial<CompileCssResult>[];
cssTransformErrors: Error[];
}): TransformOptions['preprocessStyle'] {
let processedStylesCount = 0;
Expand All @@ -24,9 +26,10 @@ export function createStylePreprocessor({
try {
const result = await preprocessCSS(content, id, viteConfig);

if (result.deps) {
cssDeps[index] = [...result.deps].map((dep) => normalizePath(dep));
}
cssPartialCompileResults[index] = {
isGlobal: !!attrs['is:global'],
dependencies: result.deps ? [...result.deps].map((dep) => normalizePath(dep)) : [],
};

let map: string | undefined;
if (result.map) {
Expand Down
12 changes: 12 additions & 0 deletions packages/astro/src/core/compile/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,15 @@ export type TransformStyle = (
source: string,
lang: string
) => TransformStyleResult | Promise<TransformStyleResult>;

export interface CompileCssResult {
code: string;
/**
* Whether this is `<style is:global>`
*/
isGlobal: boolean;
/**
* The dependencies of the transformed CSS (Normalized paths)
bluwy marked this conversation as resolved.
Show resolved Hide resolved
*/
dependencies: string[];
}
23 changes: 20 additions & 3 deletions packages/astro/src/vite-plugin-astro/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import type { SourceDescription } from 'rollup';
import type * as vite from 'vite';
import type { AstroConfig, AstroSettings } from '../@types/astro.js';
import type { Logger } from '../core/logger/core.js';
import type { CompileMetadata, PluginMetadata as AstroPluginMetadata } from './types.js';
import type {
CompileMetadata,
PluginCssMetadata as AstroPluginCssMetadata,
PluginMetadata as AstroPluginMetadata,
} from './types.js';

import { normalizePath } from 'vite';
import { normalizeFilename } from '../vite-plugin-utils/index.js';
Expand All @@ -11,7 +15,7 @@ import { handleHotUpdate } from './hmr.js';
import { parseAstroRequest } from './query.js';
import { loadId } from './utils.js';
export { getAstroMetadata } from './metadata.js';
export type { AstroPluginMetadata };
export type { AstroPluginMetadata, AstroPluginCssMetadata };

interface AstroPluginOptions {
settings: AstroSettings;
Expand Down Expand Up @@ -116,7 +120,20 @@ export default function astro({ settings, logger }: AstroPluginOptions): vite.Pl
// Register dependencies from preprocessing this style
result.dependencies?.forEach((dep) => this.addWatchFile(dep));

return { code: result.code };
return {
code: result.code,
// This metadata is used by `cssScopeToPlugin` to remove this module from the bundle
// if the `filename` default export (the Astro component) is unused.
meta: result.isGlobal
? undefined
: ({
astroCss: {
cssScopeTo: {
[filename]: ['default'],
},
},
} satisfies AstroPluginCssMetadata),
};
}
case 'script': {
if (typeof query.index === 'undefined') {
Expand Down
23 changes: 22 additions & 1 deletion packages/astro/src/vite-plugin-astro/types.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { HoistedScript, TransformResult } from '@astrojs/compiler';
import type { PropagationHint } from '../@types/astro.js';
import type { CompileCssResult } from '../core/compile/compile.js';
import type { CompileCssResult } from '../core/compile/types.js';

export interface PageOptions {
prerender?: boolean;
Expand All @@ -17,6 +17,27 @@ export interface PluginMetadata {
};
}

export interface PluginCssMetadata {
astroCss: {
/**
* For Astro CSS virtual modules, it can scope to the main Astro module's default export
* so that if those exports are treeshaken away, the CSS module will also be treeshaken.
*
* Example config if the CSS id is `/src/Foo.astro?astro&type=style&lang.css`:
* ```js
* cssScopeTo: {
* '/src/Foo.astro': ['default']
* }
* ```
*
* The above is the only config we use today, but we're exposing as a `Record` to follow the
* upstream Vite implementation: https://github.com/vitejs/vite/pull/16058. When/If that lands,
* we can also remove our custom implementation.
*/
cssScopeTo: Record<string, string[]>;
};
}

export interface CompileMetadata {
/** Used for HMR to compare code changes */
originalCode: string;
Expand Down
10 changes: 10 additions & 0 deletions packages/astro/test/0-css.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,16 @@ describe('CSS', function () {
it('<style lang="scss">', async () => {
assert.match(bundledCSS, /h1\[data-astro-cid-[^{]*\{color:#ff69b4\}/);
});

it('Styles through barrel files should only include used Astro scoped styles', async () => {
const barrelHtml = await fixture.readFile('/barrel-styles/index.html');
const barrel$ = cheerio.load(barrelHtml);
const barrelBundledCssHref = barrel$('link[rel=stylesheet][href^=/_astro/]').attr('href');
const style = await fixture.readFile(barrelBundledCssHref.replace(/^\/?/, '/'));
assert.match(style, /\.comp-a\[data-astro-cid/);
assert.match(style, /\.comp-c\{/);
assert.doesNotMatch(style, /\.comp-b/);
});
});

describe('Styles in src/', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<p class="comp-a">A</p>

<style>
.comp-a {
color: red;
}
</style>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<p class="comp-b">B</p>

<style>
.comp-b {
color: red;
}
</style>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<p class="comp-c">C</p>

<style is:global>
.comp-c {
color: red;
}
</style>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export { default as A } from './A.astro';
export { default as B } from './B.astro';
export { default as C } from './C.astro';
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
import { A } from './_components';
---

<A />

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import "../styles/Three.css"
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
import '../components/One.astro';
import '../components/Two.astro';
await import('../components/Three.astro');
await import('../components/Three.js');
Copy link
Member Author

Choose a reason for hiding this comment

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

This PR caused this test to fail because the Three.astro component was imported but it's never used, hence the scoped styles in Three.astro weren't injected. Previously it was testing if the scoped styles were injected which I don't think it's quite right.

This test comes from #6176, so to stay true to what it's testing, I changed it to a .js file so that the CSS is no longer scoped.

---
<html>
<head>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
body { background: yellow;}
Loading