Skip to content

Commit d109a43

Browse files
xiaoxiaojxclaude
xiaoxiaojx
and
claude
authored
fix: multiple bugfixes and optimizations in CSS modules (#20648)
- Fix third code point position in walkCssTokens number detection (pos → pos+1) - Fix multiline CSS comment regex to match across line breaks - Fix swapped :import/:export in error message - Fix comma callback incorrectly popping balanced stack, causing stray ')' in output - Fix cache comparison missing inheritance array length check - Fix match.index mutation side effect in publicPathAutoRegex replacement - Fix typo GRID_TEMPLATE_ARES → GRID_TEMPLATE_AREAS - Remove duplicate grid-column-start property setting - Move publicPathAutoRegex to module scope to avoid repeated construction - Precompute merged callbacks in consumeUntil to avoid per-token object spread - Simplify redundant nested ternary in CssGenerator - Replace spread+reduce with for-of loop for Map→Object conversion - Merge duplicate getCompilationHooks calls in CssLoadingRuntimeModule Co-Authored-By: Claude Opus 4.6 <[email protected]> * test: update snapshots for CSS module bugfixes Update walkCssTokens snapshot for correct number token parsing (-.1, +.1 now correctly recognized as number tokens instead of delim + number). Update postcss-modules-plugins snapshots for correct :local() comma handling (stray ')' no longer in output). Co-Authored-By: Claude Opus 4.6 <[email protected]> * test: add multiline CSS comment test cases for @value rule Add three test cases with multiline comments in @value declarations: - @value with multiline comments around name, colon, and value - @value import with multiline comments around name, from, and request - @value empty with multiline comment as value These cover the CSS_COMMENT regex fix (.*? → [\s\S]*?) ensuring multiline comments are correctly stripped during @value parsing. Co-Authored-By: Claude Opus 4.6 <[email protected]> * test: update ConfigCacheTestCases snapshot for multiline CSS comment tests Co-Authored-By: Claude Opus 4.6 <[email protected]> * test: remove obsolete ConfigCacheTestCases snapshots Clean up 94 obsolete snapshots for chunks-order, import-meta-env, and extract-source-map-css test cases. Co-Authored-By: Claude Opus 4.6 <[email protected]> * update * Revert "test: remove obsolete ConfigCacheTestCases snapshots" This reverts commit 652e2f0. * test: update ConfigCacheTestCases CSS snapshots Update postcss-modules-plugins snapshots reflecting the comma balanced stack fix (removal of stray ')' in :local() output). Add missing CSS test snapshots for ConfigCacheTestCases. Co-Authored-By: Claude Opus 4.6 <[email protected]> * update --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
1 parent 474b377 commit d109a43

10 files changed

Lines changed: 386 additions & 64 deletions

‎.changeset/fix-css-code-review.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"webpack": patch
3+
---
4+
5+
Fix multiple bugs and optimizations in CSS modules: correct third code point position in walkCssTokens number detection, fix multiline CSS comment regex, fix swapped :import/:export error message, fix comma callback incorrectly popping balanced stack, fix cache comparison missing array length check, fix match.index mutation side effect, move publicPathAutoRegex to module scope, precompute merged callbacks in consumeUntil, simplify redundant ternary in CssGenerator, fix typo GRID_TEMPLATE_ARES, remove duplicate grid-column-start, and merge duplicate getCompilationHooks calls.

‎lib/css/CssGenerator.js‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -275,10 +275,8 @@ class CssGenerator extends Generator {
275275
generate(module, generateContext) {
276276
const exportType = /** @type {CssModule} */ (module).exportType || "link";
277277
const source =
278-
generateContext.type === JAVASCRIPT_TYPE
279-
? exportType === "link"
280-
? new ReplaceSource(new RawSource(""))
281-
: new ReplaceSource(/** @type {Source} */ (module.originalSource()))
278+
generateContext.type === JAVASCRIPT_TYPE && exportType === "link"
279+
? new ReplaceSource(new RawSource(""))
282280
: new ReplaceSource(/** @type {Source} */ (module.originalSource()));
283281
/** @type {InitFragment<GenerateContext>[]} */
284282
const initFragments = [];
@@ -616,12 +614,12 @@ class CssGenerator extends Generator {
616614
return 0;
617615
}
618616
const exports = cssData.exports;
619-
const stringifiedExports = JSON.stringify(
620-
[...exports].reduce((obj, [key, value]) => {
621-
obj[key] = value;
622-
return obj;
623-
}, /** @type {Record<string, string>} */ ({}))
624-
);
617+
/** @type {Record<string, string>} */
618+
const exportsObj = {};
619+
for (const [key, value] of exports) {
620+
exportsObj[key] = value;
621+
}
622+
const stringifiedExports = JSON.stringify(exportsObj);
625623

626624
return stringifiedExports.length + 42;
627625
}

‎lib/css/CssLoadingRuntimeModule.js‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,16 +121,13 @@ class CssLoadingRuntimeModule extends RuntimeModule {
121121
(environment.document || isNeutralPlatform) &&
122122
chunk.hasChildByOrder(chunkGraph, "preload", true, chunkHasCss);
123123

124-
const { linkPreload, linkPrefetch } =
124+
const { linkPreload, linkPrefetch, createStylesheet } =
125125
CssLoadingRuntimeModule.getCompilationHooks(compilation);
126126

127127
const withFetchPriority = _runtimeRequirements.has(
128128
RuntimeGlobals.hasFetchPriority
129129
);
130130

131-
const { createStylesheet } =
132-
CssLoadingRuntimeModule.getCompilationHooks(compilation);
133-
134131
const stateExpression = withHmr
135132
? `${RuntimeGlobals.hmrRuntimeStatePrefix}_css`
136133
: undefined;

‎lib/css/CssModulesPlugin.js‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ const removeBOM = require("../util/removeBOM");
4444
const CssGenerator = require("./CssGenerator");
4545
const CssParser = require("./CssParser");
4646

47+
const publicPathAutoRegex = new RegExp(CssUrlDependency.PUBLIC_PATH_AUTO, "g");
48+
4749
/** @typedef {import("webpack-sources").Source} Source */
4850
/** @typedef {import("../config/defaults").OutputNormalizedWithDefaults} OutputOptions */
4951
/** @typedef {import("../Chunk")} Chunk */
@@ -455,13 +457,13 @@ class CssModulesPlugin {
455457
return source;
456458
}
457459
const exports = cssData.exports;
460+
/** @type {Record<string, string>} */
461+
const exportsObj = {};
462+
for (const [key, value] of exports) {
463+
exportsObj[key] = value;
464+
}
458465
const stringifiedExports = JSON.stringify(
459-
JSON.stringify(
460-
[...exports].reduce((obj, [key, value]) => {
461-
obj[key] = value;
462-
return obj;
463-
}, /** @type {Record<string, string>} */ ({}))
464-
)
466+
JSON.stringify(exportsObj)
465467
);
466468

467469
const hmrCode = Template.asString([
@@ -848,6 +850,7 @@ class CssModulesPlugin {
848850
if (
849851
cacheEntry &&
850852
cacheEntry.undoPath === undoPath &&
853+
cacheEntry.inheritance.length === inheritance.length &&
851854
cacheEntry.inheritance.every(([layer, supports, media], i) => {
852855
const item = inheritance[i];
853856
if (Array.isArray(item)) {
@@ -862,18 +865,15 @@ class CssModulesPlugin {
862865
const moduleSourceCode =
863866
/** @type {string} */
864867
(moduleSourceContent.source());
865-
const publicPathAutoRegex = new RegExp(
866-
CssUrlDependency.PUBLIC_PATH_AUTO,
867-
"g"
868-
);
868+
publicPathAutoRegex.lastIndex = 0;
869869
/** @type {Source} */
870870
let moduleSource = new ReplaceSource(moduleSourceContent);
871871
/** @type {null | RegExpExecArray} */
872872
let match;
873873
while ((match = publicPathAutoRegex.exec(moduleSourceCode))) {
874874
/** @type {ReplaceSource} */ (moduleSource).replace(
875875
match.index,
876-
(match.index += match[0].length - 1),
876+
match.index + match[0].length - 1,
877877
undoPath
878878
);
879879
}

‎lib/css/CssParser.js‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ const IMAGE_SET_FUNCTION = /^(?:-\w+-)?image-set$/i;
6262
const OPTIONALLY_VENDOR_PREFIXED_KEYFRAMES_AT_RULE = /^@(?:-\w+-)?keyframes$/;
6363
const COMPOSES_PROPERTY = /^(?:composes|compose-with)$/i;
6464
const IS_MODULES = /\.modules?\.[^.]+$/i;
65-
const CSS_COMMENT = /\/\*((?!\*\/).*?)\*\//g;
65+
const CSS_COMMENT = /\/\*((?!\*\/)[\s\S]*?)\*\//g;
6666

6767
/**
6868
* @param {RegExp} regexp a regexp
@@ -371,7 +371,7 @@ const GRID_AUTO_FLOW = {
371371
};
372372

373373
/** @type {Record<string, number>} */
374-
const GRID_TEMPLATE_ARES = {
374+
const GRID_TEMPLATE_AREAS = {
375375
// Special
376376
none: 1,
377377
...GLOBAL_VALUES
@@ -390,7 +390,7 @@ const GRID_TEMPLATE_COLUMNS_OR_ROWS = {
390390

391391
/** @type {Record<string, number>} */
392392
const GRID_TEMPLATE = {
393-
...GRID_TEMPLATE_ARES,
393+
...GRID_TEMPLATE_AREAS,
394394
...GRID_TEMPLATE_COLUMNS_OR_ROWS
395395
};
396396

@@ -400,7 +400,7 @@ const GRID = {
400400
dense: 1,
401401
...GRID_AUTO_COLUMNS_OR_ROW,
402402
...GRID_AUTO_FLOW,
403-
...GRID_TEMPLATE_ARES,
403+
...GRID_TEMPLATE_AREAS,
404404
...GRID_TEMPLATE_COLUMNS_OR_ROWS
405405
};
406406

@@ -512,12 +512,11 @@ const getKnownProperties = (options = {}) => {
512512
knownProperties.set("grid-column", GRID_AREA_OR_COLUMN_OR_ROW);
513513
knownProperties.set("grid-column-end", GRID_AREA_OR_COLUMN_OR_ROW);
514514
knownProperties.set("grid-column-start", GRID_AREA_OR_COLUMN_OR_ROW);
515-
knownProperties.set("grid-column-start", GRID_AREA_OR_COLUMN_OR_ROW);
516515
knownProperties.set("grid-row", GRID_AREA_OR_COLUMN_OR_ROW);
517516
knownProperties.set("grid-row-end", GRID_AREA_OR_COLUMN_OR_ROW);
518517
knownProperties.set("grid-row-start", GRID_AREA_OR_COLUMN_OR_ROW);
519518
knownProperties.set("grid-template", GRID_TEMPLATE);
520-
knownProperties.set("grid-template-areas", GRID_TEMPLATE_ARES);
519+
knownProperties.set("grid-template-areas", GRID_TEMPLATE_AREAS);
521520
knownProperties.set("grid-template-columns", GRID_TEMPLATE_COLUMNS_OR_ROWS);
522521
knownProperties.set("grid-template-rows", GRID_TEMPLATE_COLUMNS_OR_ROWS);
523522
}
@@ -1105,7 +1104,7 @@ class CssParser extends Parser {
11051104
if (!str) {
11061105
this._emitWarning(
11071106
state,
1108-
`Unexpected '${input[pos]}' at ${pos} during parsing of '${type ? ":import" : ":export"}' (expected string)`,
1107+
`Unexpected '${input[pos]}' at ${pos} during parsing of '${type === 0 ? ":import" : ":export"}' (expected string)`,
11091108
locConverter,
11101109
stringStart,
11111110
pos
@@ -2490,13 +2489,9 @@ class CssParser extends Parser {
24902489
return end;
24912490
},
24922491
comma: (input, start, end) => {
2493-
if (isModules) {
2494-
const popped = balanced.pop();
2495-
2496-
if (!popped) {
2497-
// Reset stack for `:global .class :local .class-other` selector after
2498-
modeData = undefined;
2499-
}
2492+
if (isModules && balanced.length === 0) {
2493+
// Reset stack for `:global .class :local .class-other` selector after
2494+
modeData = undefined;
25002495
}
25012496

25022497
lastTokenEndForComments = start;

‎lib/css/walkCssTokens.js‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ const _ifThreeCodePointsWouldStartANumber = (input, pos, f, s, t) => {
469469

470470
const first = f || input.charCodeAt(pos - 1);
471471
const second = s || input.charCodeAt(pos);
472-
const third = t || input.charCodeAt(pos);
472+
const third = t || input.charCodeAt(pos + 1);
473473

474474
// Look at the first code point:
475475

@@ -1382,12 +1382,14 @@ const consumeUntil = (input, pos, callbacks, additional, options = {}) => {
13821382
};
13831383
}
13841384

1385+
const mergedCallbacks = { ...servicedCallbacks, ...callbacks };
1386+
13851387
while (pos < input.length) {
13861388
// Consume comments.
13871389
pos = consumeComments(
13881390
input,
13891391
pos,
1390-
needHandle ? { ...servicedCallbacks, ...callbacks } : servicedCallbacks
1392+
needHandle ? mergedCallbacks : servicedCallbacks
13911393
);
13921394

13931395
const start = pos;
@@ -1397,7 +1399,7 @@ const consumeUntil = (input, pos, callbacks, additional, options = {}) => {
13971399
pos = consumeAToken(
13981400
input,
13991401
pos,
1400-
needHandle ? { ...servicedCallbacks, ...callbacks } : servicedCallbacks
1402+
needHandle ? mergedCallbacks : servicedCallbacks
14011403
);
14021404

14031405
if (needTerminate) {

0 commit comments

Comments
 (0)