Skip to content

Commit 662f06f

Browse files
authoredDec 27, 2023
Fix duplicated CSS modules inlining (#9531)
* Fix duplicated CSS modules inlining * Remove unused mode param
1 parent 7224809 commit 662f06f

File tree

6 files changed

+57
-43
lines changed

6 files changed

+57
-43
lines changed
 

‎.changeset/blue-ladybugs-march.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'astro': patch
3+
---
4+
5+
Fixes duplicated CSS modules content when it's imported by both Astro files and framework components

‎packages/astro/src/content/vite-plugin-content-assets.ts

+1-5
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,7 @@ export function astroContentAssetPropagationPlugin({
6464
if (!devModuleLoader.getModuleById(basePath)?.ssrModule) {
6565
await devModuleLoader.import(basePath);
6666
}
67-
const { styles, urls } = await getStylesForURL(
68-
pathToFileURL(basePath),
69-
devModuleLoader,
70-
'development'
71-
);
67+
const { styles, urls } = await getStylesForURL(pathToFileURL(basePath), devModuleLoader);
7268

7369
const hoistedScripts = await getScriptsForURL(
7470
pathToFileURL(basePath),

‎packages/astro/src/vite-plugin-astro-server/css.ts

+33-23
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import type { RuntimeMode } from '../@types/astro.js';
21
import type { ModuleLoader } from '../core/module-loader/index.js';
32
import { viteID } from '../core/util.js';
43
import { isBuildableCSSRequest } from './util.js';
@@ -13,37 +12,48 @@ interface ImportedStyle {
1312
/** Given a filePath URL, crawl Vite’s module graph to find all style imports. */
1413
export async function getStylesForURL(
1514
filePath: URL,
16-
loader: ModuleLoader,
17-
mode: RuntimeMode
15+
loader: ModuleLoader
1816
): Promise<{ urls: Set<string>; styles: ImportedStyle[] }> {
1917
const importedCssUrls = new Set<string>();
2018
// Map of url to injected style object. Use a `url` key to deduplicate styles
2119
const importedStylesMap = new Map<string, ImportedStyle>();
2220

2321
for await (const importedModule of crawlGraph(loader, viteID(filePath), true)) {
2422
if (isBuildableCSSRequest(importedModule.url)) {
25-
let ssrModule: Record<string, any>;
26-
try {
27-
// The SSR module is possibly not loaded. Load it if it's null.
28-
ssrModule = importedModule.ssrModule ?? (await loader.import(importedModule.url));
29-
} catch {
30-
// The module may not be inline-able, e.g. SCSS partials. Skip it as it may already
31-
// be inlined into other modules if it happens to be in the graph.
32-
continue;
23+
// In dev, we inline all styles if possible
24+
let css = '';
25+
// If this is a plain CSS module, the default export should be a string
26+
if (typeof importedModule.ssrModule?.default === 'string') {
27+
css = importedModule.ssrModule.default;
3328
}
34-
if (
35-
mode === 'development' && // only inline in development
36-
typeof ssrModule?.default === 'string' // ignore JS module styles
37-
) {
38-
importedStylesMap.set(importedModule.url, {
39-
id: importedModule.id ?? importedModule.url,
40-
url: importedModule.url,
41-
content: ssrModule.default,
42-
});
43-
} else {
44-
// NOTE: We use the `url` property here. `id` would break Windows.
45-
importedCssUrls.add(importedModule.url);
29+
// Else try to load it
30+
else {
31+
const url = new URL(importedModule.url, 'http://localhost');
32+
// Mark url with ?inline so Vite will return the CSS as plain string, even for CSS modules
33+
url.searchParams.set('inline', '');
34+
const modId = `${decodeURI(url.pathname)}${url.search}`;
35+
36+
try {
37+
// The SSR module is possibly not loaded. Load it if it's null.
38+
const ssrModule = await loader.import(modId);
39+
css = ssrModule.default;
40+
} catch {
41+
// Some CSS modules, e.g. from Vue files, may not work with the ?inline query.
42+
// If so, we fallback to a url instead
43+
if (modId.includes('.module.')) {
44+
importedCssUrls.add(importedModule.url);
45+
}
46+
// The module may not be inline-able, e.g. SCSS partials. Skip it as it may already
47+
// be inlined into other modules if it happens to be in the graph.
48+
continue;
49+
}
4650
}
51+
52+
importedStylesMap.set(importedModule.url, {
53+
id: importedModule.id ?? importedModule.url,
54+
url: importedModule.url,
55+
content: css,
56+
});
4757
}
4858
}
4959

‎packages/astro/src/vite-plugin-astro-server/route.ts

+1-5
Original file line numberDiff line numberDiff line change
@@ -436,11 +436,7 @@ async function getScriptsAndStyles({ pipeline, filePath }: GetScriptsAndStylesPa
436436
}
437437

438438
// Pass framework CSS in as style tags to be appended to the page.
439-
const { urls: styleUrls, styles: importedStyles } = await getStylesForURL(
440-
filePath,
441-
moduleLoader,
442-
mode
443-
);
439+
const { urls: styleUrls, styles: importedStyles } = await getStylesForURL(filePath, moduleLoader);
444440
let links = new Set<SSRElement>();
445441
[...styleUrls].forEach((href) => {
446442
links.add({

‎packages/astro/test/0-css.test.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ describe('CSS', function () {
367367
'ReactModules.module.sass',
368368
];
369369
for (const style of styles) {
370-
const href = $(`link[href$="${style}"]`).attr('href');
370+
const href = $(`style[data-vite-dev-id$="${style}"]`).attr('data-vite-dev-id');
371371
expect((await fixture.fetch(href)).status, style).to.equal(200);
372372
}
373373

@@ -423,7 +423,7 @@ describe('CSS', function () {
423423

424424
it('.module.css ordering', () => {
425425
const globalStyleTag = $('style[data-vite-dev-id$="default.css"]');
426-
const moduleStyleTag = $('link[href$="ModuleOrdering.module.css"]');
426+
const moduleStyleTag = $('style[data-vite-dev-id$="ModuleOrdering.module.css"]');
427427
const globalStyleClassIndex = globalStyleTag.index();
428428
const moduleStyleClassIndex = moduleStyleTag.index();
429429
// css module has higher priority than global style

‎packages/astro/test/units/dev/styles.test.js

+15-8
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,17 @@ class TestLoader {
1515
getModulesByFile(id) {
1616
return this.modules.has(id) ? [this.modules.get(id)] : [];
1717
}
18+
import(id) {
19+
// try to normalize inline CSS requests so we can map to the existing modules value
20+
id = id.replace(/(\?|&)inline=?(&|$)/, (_, start, end) => (end ? start : '')).replace(/=$/, '');
21+
for (const mod of this.modules.values()) {
22+
for (const importedMod of mod.importedModules) {
23+
if (importedMod.id === id) {
24+
return importedMod.ssrModule;
25+
}
26+
}
27+
}
28+
}
1829
}
1930

2031
describe('Crawling graph for CSS', () => {
@@ -35,7 +46,7 @@ describe('Crawling graph for CSS', () => {
3546
id: indexId + '?astro&style.css',
3647
url: indexId + '?astro&style.css',
3748
importers: new Set([{ id: indexId }]),
38-
ssrModule: {},
49+
ssrModule: { default: '.index {}' },
3950
},
4051
],
4152
importers: new Set(),
@@ -50,7 +61,7 @@ describe('Crawling graph for CSS', () => {
5061
id: aboutId + '?astro&style.css',
5162
url: aboutId + '?astro&style.css',
5263
importers: new Set([{ id: aboutId }]),
53-
ssrModule: {},
64+
ssrModule: { default: '.about {}' },
5465
},
5566
],
5667
importers: new Set(),
@@ -64,11 +75,7 @@ describe('Crawling graph for CSS', () => {
6475
it("importedModules is checked against the child's importers", async () => {
6576
// In dev mode, HMR modules tracked are added to importedModules. We use `importers`
6677
// to verify that they are true importers.
67-
const res = await getStylesForURL(
68-
new URL('./src/pages/index.astro', root),
69-
loader,
70-
'development'
71-
);
72-
expect(res.urls.size).to.equal(1);
78+
const res = await getStylesForURL(new URL('./src/pages/index.astro', root), loader);
79+
expect(res.styles.length).to.equal(1);
7380
});
7481
});

0 commit comments

Comments
 (0)
Please sign in to comment.