From 79aa10a7130e9d36fb53e4b1ba1d97aa96136d4c Mon Sep 17 00:00:00 2001 From: bluwy Date: Wed, 27 Dec 2023 22:07:03 +0800 Subject: [PATCH 1/2] Fix duplicated CSS modules inlining --- .changeset/blue-ladybugs-march.md | 5 ++ .../astro/src/vite-plugin-astro-server/css.ts | 56 ++++++++++++------- packages/astro/test/0-css.test.js | 4 +- packages/astro/test/units/dev/styles.test.js | 17 +++++- 4 files changed, 58 insertions(+), 24 deletions(-) create mode 100644 .changeset/blue-ladybugs-march.md diff --git a/.changeset/blue-ladybugs-march.md b/.changeset/blue-ladybugs-march.md new file mode 100644 index 000000000000..4394b6ad35e2 --- /dev/null +++ b/.changeset/blue-ladybugs-march.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes duplicated CSS modules content when it's imported by both Astro files and framework components diff --git a/packages/astro/src/vite-plugin-astro-server/css.ts b/packages/astro/src/vite-plugin-astro-server/css.ts index 0da51db1e870..2034a7c9614b 100644 --- a/packages/astro/src/vite-plugin-astro-server/css.ts +++ b/packages/astro/src/vite-plugin-astro-server/css.ts @@ -22,28 +22,46 @@ export async function getStylesForURL( for await (const importedModule of crawlGraph(loader, viteID(filePath), true)) { if (isBuildableCSSRequest(importedModule.url)) { - let ssrModule: Record; - try { - // The SSR module is possibly not loaded. Load it if it's null. - ssrModule = importedModule.ssrModule ?? (await loader.import(importedModule.url)); - } catch { - // The module may not be inline-able, e.g. SCSS partials. Skip it as it may already - // be inlined into other modules if it happens to be in the graph. + // In production, we can simply assign the styles as URLs + if (mode !== 'development') { + importedCssUrls.add(importedModule.url); continue; } - if ( - mode === 'development' && // only inline in development - typeof ssrModule?.default === 'string' // ignore JS module styles - ) { - importedStylesMap.set(importedModule.url, { - id: importedModule.id ?? importedModule.url, - url: importedModule.url, - content: ssrModule.default, - }); - } else { - // NOTE: We use the `url` property here. `id` would break Windows. - importedCssUrls.add(importedModule.url); + + // In dev, we inline all styles if possible + let css = ''; + // If this is a plain CSS module, the default export should be a string + if (typeof importedModule.ssrModule?.default === 'string') { + css = importedModule.ssrModule.default; } + // Else try to load it + else { + const url = new URL(importedModule.url, 'http://localhost'); + // Mark url with ?inline so Vite will return the CSS as plain string, even for CSS modules + url.searchParams.set('inline', ''); + const modId = `${decodeURI(url.pathname)}${url.search}`; + + try { + // The SSR module is possibly not loaded. Load it if it's null. + const ssrModule = await loader.import(modId); + css = ssrModule.default; + } catch { + // Some CSS modules, e.g. from Vue files, may not work with the ?inline query. + // If so, we fallback to a url instead + if (modId.includes('.module.')) { + importedCssUrls.add(importedModule.url); + } + // The module may not be inline-able, e.g. SCSS partials. Skip it as it may already + // be inlined into other modules if it happens to be in the graph. + continue; + } + } + + importedStylesMap.set(importedModule.url, { + id: importedModule.id ?? importedModule.url, + url: importedModule.url, + content: css, + }); } } diff --git a/packages/astro/test/0-css.test.js b/packages/astro/test/0-css.test.js index 2120452eb53a..3a4b9241dcf0 100644 --- a/packages/astro/test/0-css.test.js +++ b/packages/astro/test/0-css.test.js @@ -367,7 +367,7 @@ describe('CSS', function () { 'ReactModules.module.sass', ]; for (const style of styles) { - const href = $(`link[href$="${style}"]`).attr('href'); + const href = $(`style[data-vite-dev-id$="${style}"]`).attr('data-vite-dev-id'); expect((await fixture.fetch(href)).status, style).to.equal(200); } @@ -423,7 +423,7 @@ describe('CSS', function () { it('.module.css ordering', () => { const globalStyleTag = $('style[data-vite-dev-id$="default.css"]'); - const moduleStyleTag = $('link[href$="ModuleOrdering.module.css"]'); + const moduleStyleTag = $('style[data-vite-dev-id$="ModuleOrdering.module.css"]'); const globalStyleClassIndex = globalStyleTag.index(); const moduleStyleClassIndex = moduleStyleTag.index(); // css module has higher priority than global style diff --git a/packages/astro/test/units/dev/styles.test.js b/packages/astro/test/units/dev/styles.test.js index 76b38dd4372b..3e7d283ff69f 100644 --- a/packages/astro/test/units/dev/styles.test.js +++ b/packages/astro/test/units/dev/styles.test.js @@ -15,6 +15,17 @@ class TestLoader { getModulesByFile(id) { return this.modules.has(id) ? [this.modules.get(id)] : []; } + import(id) { + // try to normalize inline CSS requests so we can map to the existing modules value + id = id.replace(/(\?|&)inline=?(&|$)/, (_, start, end) => (end ? start : '')).replace(/=$/, ''); + for (const mod of this.modules.values()) { + for (const importedMod of mod.importedModules) { + if (importedMod.id === id) { + return importedMod.ssrModule; + } + } + } + } } describe('Crawling graph for CSS', () => { @@ -35,7 +46,7 @@ describe('Crawling graph for CSS', () => { id: indexId + '?astro&style.css', url: indexId + '?astro&style.css', importers: new Set([{ id: indexId }]), - ssrModule: {}, + ssrModule: { default: '.index {}' }, }, ], importers: new Set(), @@ -50,7 +61,7 @@ describe('Crawling graph for CSS', () => { id: aboutId + '?astro&style.css', url: aboutId + '?astro&style.css', importers: new Set([{ id: aboutId }]), - ssrModule: {}, + ssrModule: { default: '.about {}' }, }, ], importers: new Set(), @@ -69,6 +80,6 @@ describe('Crawling graph for CSS', () => { loader, 'development' ); - expect(res.urls.size).to.equal(1); + expect(res.styles.length).to.equal(1); }); }); From f157c451510c0c205c488ed40fb4b2d1bfd3aa3b Mon Sep 17 00:00:00 2001 From: bluwy Date: Wed, 27 Dec 2023 22:13:12 +0800 Subject: [PATCH 2/2] Remove unused mode param --- .../astro/src/content/vite-plugin-content-assets.ts | 6 +----- packages/astro/src/vite-plugin-astro-server/css.ts | 10 +--------- packages/astro/src/vite-plugin-astro-server/route.ts | 6 +----- packages/astro/test/units/dev/styles.test.js | 6 +----- 4 files changed, 4 insertions(+), 24 deletions(-) diff --git a/packages/astro/src/content/vite-plugin-content-assets.ts b/packages/astro/src/content/vite-plugin-content-assets.ts index 5b79d5653ea7..6eeef1c40640 100644 --- a/packages/astro/src/content/vite-plugin-content-assets.ts +++ b/packages/astro/src/content/vite-plugin-content-assets.ts @@ -64,11 +64,7 @@ export function astroContentAssetPropagationPlugin({ if (!devModuleLoader.getModuleById(basePath)?.ssrModule) { await devModuleLoader.import(basePath); } - const { styles, urls } = await getStylesForURL( - pathToFileURL(basePath), - devModuleLoader, - 'development' - ); + const { styles, urls } = await getStylesForURL(pathToFileURL(basePath), devModuleLoader); const hoistedScripts = await getScriptsForURL( pathToFileURL(basePath), diff --git a/packages/astro/src/vite-plugin-astro-server/css.ts b/packages/astro/src/vite-plugin-astro-server/css.ts index 2034a7c9614b..0f0002907a88 100644 --- a/packages/astro/src/vite-plugin-astro-server/css.ts +++ b/packages/astro/src/vite-plugin-astro-server/css.ts @@ -1,4 +1,3 @@ -import type { RuntimeMode } from '../@types/astro.js'; import type { ModuleLoader } from '../core/module-loader/index.js'; import { viteID } from '../core/util.js'; import { isBuildableCSSRequest } from './util.js'; @@ -13,8 +12,7 @@ interface ImportedStyle { /** Given a filePath URL, crawl Vite’s module graph to find all style imports. */ export async function getStylesForURL( filePath: URL, - loader: ModuleLoader, - mode: RuntimeMode + loader: ModuleLoader ): Promise<{ urls: Set; styles: ImportedStyle[] }> { const importedCssUrls = new Set(); // Map of url to injected style object. Use a `url` key to deduplicate styles @@ -22,12 +20,6 @@ export async function getStylesForURL( for await (const importedModule of crawlGraph(loader, viteID(filePath), true)) { if (isBuildableCSSRequest(importedModule.url)) { - // In production, we can simply assign the styles as URLs - if (mode !== 'development') { - importedCssUrls.add(importedModule.url); - continue; - } - // In dev, we inline all styles if possible let css = ''; // If this is a plain CSS module, the default export should be a string diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index 3196b951a796..e37850acce24 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -436,11 +436,7 @@ async function getScriptsAndStyles({ pipeline, filePath }: GetScriptsAndStylesPa } // Pass framework CSS in as style tags to be appended to the page. - const { urls: styleUrls, styles: importedStyles } = await getStylesForURL( - filePath, - moduleLoader, - mode - ); + const { urls: styleUrls, styles: importedStyles } = await getStylesForURL(filePath, moduleLoader); let links = new Set(); [...styleUrls].forEach((href) => { links.add({ diff --git a/packages/astro/test/units/dev/styles.test.js b/packages/astro/test/units/dev/styles.test.js index 3e7d283ff69f..526b8fbef9f8 100644 --- a/packages/astro/test/units/dev/styles.test.js +++ b/packages/astro/test/units/dev/styles.test.js @@ -75,11 +75,7 @@ describe('Crawling graph for CSS', () => { it("importedModules is checked against the child's importers", async () => { // In dev mode, HMR modules tracked are added to importedModules. We use `importers` // to verify that they are true importers. - const res = await getStylesForURL( - new URL('./src/pages/index.astro', root), - loader, - 'development' - ); + const res = await getStylesForURL(new URL('./src/pages/index.astro', root), loader); expect(res.styles.length).to.equal(1); }); });