Skip to content

Commit 7ee169b

Browse files
aryanraj45aryanraj45
committed
fix(css): take out the declaration a lowered shorthand writes over
A shorthand the target cannot read is lowered into the line it keeps plus the longhands beside it, and that line is a value every target reads. The drop read the shorthand by its source spelling, so an earlier declaration of the property looked like the fallback the author wrote and stayed: `text-decoration:underline;text-decoration:underline dotted` kept both where a second pass took one out. Read by what is written instead.
1 parent 7e6cb67 commit 7ee169b

9 files changed

Lines changed: 215 additions & 1 deletion

File tree

‎.changeset/014-css-lowered-shorthand-dead-write.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+
Take out the declaration a lowered shorthand writes over, which its source spelling made look like a fallback.

‎lib/css/syntax-parser.js‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11630,7 +11630,7 @@ const _writeIsDead = (path, writer, earlier, later, keyframe) => {
1163011630
return _laterWriteWins(
1163111631
path,
1163211632
later,
11633-
_printedValue(laterText, laterColon),
11633+
_writtenValue(laterText, laterColon, property),
1163411634
earlier,
1163511635
_printedValue(earlierText, earlierColon),
1163611636
property,
@@ -13629,6 +13629,15 @@ const _withColorFallback = (one) => {
1362913629
return fallback === null ? one : fallback + one;
1363013630
};
1363113631

13632+
// The shorthands `_expandUnsupportedShorthand` may write as something else,
13633+
// so a property that is none of them is read by what it says.
13634+
const LOWERED_SHORTHANDS = new Set([
13635+
...PLACE_SHORTHANDS,
13636+
...ONE_VALUE_PAIR_SHORTHANDS,
13637+
"inset",
13638+
"text-decoration"
13639+
]);
13640+
1363213641
/**
1363313642
* The longhand declarations a shorthand the target cannot read says, from the
1363413643
* shorthand's printed text — the inverse of the merges `_mergeBoxLonghands`
@@ -13703,6 +13712,34 @@ const _expandUnsupportedShorthand = (text) => {
1370313712
return out;
1370413713
};
1370513714

13715+
/**
13716+
* The value a later declaration is read for, which is the one it is written as.
13717+
* A shorthand the target cannot read is lowered into longhands, and the
13718+
* shorthand it keeps carries the line alone — a value every target reads. So an
13719+
* earlier declaration of that property is not the fallback its spelling makes
13720+
* it look like: what is written over it is readable, and it is dead.
13721+
* @param {string} text the declaration, as printed
13722+
* @param {number} colon where its `:` stands
13723+
* @param {string} property its lowercased property name
13724+
* @returns {string} the value to read it by
13725+
*/
13726+
const _writtenValue = (text, colon, property) => {
13727+
// Decided before the expansion is built: only these are ever written as
13728+
// something else, and every other property pays one lookup.
13729+
if (!LOWERED_SHORTHANDS.has(property) || !_S._transforms.lowerUnsupported) {
13730+
return _printedValue(text, colon);
13731+
}
13732+
const expanded = _expandUnsupportedShorthand(text);
13733+
if (expanded === null) return _printedValue(text, colon);
13734+
// The lowering writes the shorthand first, so its value stands in front of
13735+
// the longhands written beside it.
13736+
const at = expanded.indexOf(":");
13737+
const end = expanded.indexOf(";", at);
13738+
return at === -1 || end === -1
13739+
? _printedValue(text, colon)
13740+
: expanded.slice(at + 1, end);
13741+
};
13742+
1370613743
/**
1370713744
* Whether a declaration sets a custom property `unusedSymbols` says nothing
1370813745
* reads. Only a declaration: a rule's own name is decided by its prelude.

‎test/configCases/css/minify-lowered-shorthand-dead-write/__snapshots__/ConfigCacheTest.snap‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
2+
3+
exports[`ConfigCacheTestCases css minify-lowered-shorthand-dead-write minify-lowered-shorthand-dead-write should compile 1`] = `".a,.b,.c{text-decoration:underline;text-decoration-style:dotted}.d{& i{top:0}text-decoration:underline;text-decoration-style:dotted}.e{text-decoration:overline!important;text-decoration:underline;text-decoration-style:dotted}.f{text-decoration-line:underline;text-decoration:underline;text-decoration-style:dotted}.g{text-decoration:overline;text-decoration:var(--x)}"`;

‎test/configCases/css/minify-lowered-shorthand-dead-write/__snapshots__/ConfigTest.snap‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
2+
3+
exports[`ConfigTestCases css minify-lowered-shorthand-dead-write minify-lowered-shorthand-dead-write should compile 1`] = `".a,.b,.c{text-decoration:underline;text-decoration-style:dotted}.d{& i{top:0}text-decoration:underline;text-decoration-style:dotted}.e{text-decoration:overline!important;text-decoration:underline;text-decoration-style:dotted}.f{text-decoration-line:underline;text-decoration:underline;text-decoration-style:dotted}.g{text-decoration:overline;text-decoration:var(--x)}"`;

‎test/configCases/css/minify-lowered-shorthand-dead-write/index.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import "./style.css";
2+
3+
it("should take out the declaration a lowered shorthand writes over", () => {
4+
expect(true).toBe(true);
5+
});

‎test/configCases/css/minify-lowered-shorthand-dead-write/style.css‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/* The shorthand is lowered into the line it keeps plus a longhand beside it,
2+
and the line every target reads — so the declaration in front of it is
3+
written over rather than read as the fallback its spelling suggests. */
4+
.a { text-decoration: underline; text-decoration: underline dotted }
5+
.b { text-decoration: overline; text-decoration: underline dotted }
6+
7+
/* Every earlier one goes, not just the last. */
8+
.c { text-decoration: overline; text-decoration: blink; text-decoration: underline dotted }
9+
10+
/* A rule written between them parts nothing the later one writes over. */
11+
.d { text-decoration: overline; & i { top: 0 } text-decoration: underline dotted }
12+
13+
/* `!important` wins the tie wherever both are read. */
14+
.e { text-decoration: overline !important; text-decoration: underline dotted }
15+
16+
/* A longhand is not the property the shorthand repeats. */
17+
.f { text-decoration-line: underline; text-decoration: underline dotted }
18+
19+
/* A substitution is a value nothing here reads. */
20+
.g { text-decoration: overline; text-decoration: var(--x) }

‎test/configCases/css/minify-lowered-shorthand-dead-write/test.config.js‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
"use strict";
2+
3+
const fs = require("fs");
4+
const path = require("path");
5+
const cssMinify = require("../../../../lib/css/cssMinify");
6+
7+
module.exports = {
8+
async afterExecute(options) {
9+
const css = fs.readFileSync(
10+
path.join(options.output.path, "bundle0.css"),
11+
"utf8"
12+
);
13+
expect(css).toMatchSnapshot();
14+
15+
// All three lose the declaration written over, so all three print the one
16+
// block and are written as the one rule they are.
17+
expect(css).toContain(
18+
".a,.b,.c{text-decoration:underline;text-decoration-style:dotted}"
19+
);
20+
expect(css).toContain(
21+
".d{& i{top:0}text-decoration:underline;text-decoration-style:dotted}"
22+
);
23+
expect(css).toContain(".e{text-decoration:overline!important;");
24+
expect(css).toContain(".f{text-decoration-line:underline;");
25+
expect(css).toContain(
26+
".g{text-decoration:overline;text-decoration:var(--x)}"
27+
);
28+
29+
const again = await cssMinify({ "bundle0.css": css }, undefined, {
30+
environment: { browsers: ["safari 15"] }
31+
});
32+
expect(again.code).toBe(css);
33+
}
34+
};

‎test/configCases/css/minify-lowered-shorthand-dead-write/webpack.config.js‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
"use strict";
2+
3+
/** @type {import("../../../../").Configuration} */
4+
module.exports = {
5+
// Reads `text-decoration` as CSS 2.1's line alone, so the shorthand is
6+
// written out as the line plus the longhands beside it.
7+
target: "browserslist: safari 15",
8+
mode: "production",
9+
output: {
10+
pathinfo: false
11+
},
12+
optimization: {
13+
minimize: true,
14+
minimizer: ["..."]
15+
},
16+
experiments: {
17+
css: true
18+
}
19+
};

‎test/unitCases/CssSyntax.unittest.js‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10672,6 +10672,94 @@ describe("CssSyntax minify — a slot holding its own initial", () => {
1067210672
});
1067310673
});
1067410674

10675+
describe("CssSyntax minify — the declaration a lowered shorthand kills", () => {
10676+
// Reads `text-decoration` as CSS 2.1's line alone, so the shorthand is
10677+
// written out as the line plus the longhands beside it — and reads those.
10678+
const T = ["safari 15"];
10679+
10680+
it("takes out an earlier declaration the lowered one writes over", () => {
10681+
expect(
10682+
minifyFor("a{text-decoration:underline;text-decoration:underline dotted}", T)
10683+
).toBe("a{text-decoration:underline;text-decoration-style:dotted}");
10684+
});
10685+
10686+
it("takes it out whatever the earlier one said", () => {
10687+
expect(
10688+
minifyFor("a{text-decoration:overline;text-decoration:underline dotted}", T)
10689+
).toBe("a{text-decoration:underline;text-decoration-style:dotted}");
10690+
});
10691+
10692+
it("takes out every earlier one", () => {
10693+
expect(
10694+
minifyFor(
10695+
"a{text-decoration:overline;text-decoration:blink;text-decoration:underline dotted}",
10696+
T
10697+
)
10698+
).toBe("a{text-decoration:underline;text-decoration-style:dotted}");
10699+
});
10700+
10701+
it("takes one out past a rule written between them", () => {
10702+
expect(
10703+
minifyFor(
10704+
"a{text-decoration:overline;& b{top:0}text-decoration:underline dotted}",
10705+
T
10706+
)
10707+
).toBe("a{& b{top:0}text-decoration:underline;text-decoration-style:dotted}");
10708+
});
10709+
10710+
it("keeps the earlier one where the target reads the later whole", () => {
10711+
// Nothing is lowered, so the later value is the one a target may not read
10712+
// — and the earlier one is the fallback the author wrote.
10713+
const css = "a{text-decoration:overline;text-decoration:underline dotted}";
10714+
expect(minifyFor(css, ["chrome 130"])).toBe(css);
10715+
});
10716+
10717+
it("keeps it where the longhands the lowering writes are unread too", () => {
10718+
// The lowered form is no more readable than the shorthand was, so the
10719+
// earlier declaration is still the fallback it was written as.
10720+
expect(
10721+
minifyFor(
10722+
"a{text-decoration:overline;text-decoration:underline dotted}",
10723+
["ie 11"]
10724+
)
10725+
).toBe(
10726+
"a{text-decoration:overline;text-decoration:underline;text-decoration-style:dotted}"
10727+
);
10728+
});
10729+
10730+
it("keeps an `!important` earlier one, which wins the tie", () => {
10731+
expect(
10732+
minifyFor(
10733+
"a{text-decoration:overline!important;text-decoration:underline dotted}",
10734+
T
10735+
)
10736+
).toBe(
10737+
"a{text-decoration:overline!important;text-decoration:underline;text-decoration-style:dotted}"
10738+
);
10739+
});
10740+
10741+
it("keeps an earlier longhand, which the shorthand does not repeat", () => {
10742+
expect(
10743+
minifyFor(
10744+
"a{text-decoration-line:underline;text-decoration:underline dotted}",
10745+
T
10746+
)
10747+
).toBe(
10748+
"a{text-decoration-line:underline;text-decoration:underline;text-decoration-style:dotted}"
10749+
);
10750+
});
10751+
10752+
it("keeps an earlier one where the later is a substitution", () => {
10753+
const css = "a{text-decoration:overline;text-decoration:var(--x)}";
10754+
expect(minifyFor(css, T)).toBe(css);
10755+
});
10756+
10757+
it("keeps both with the rewrite off", () => {
10758+
const css = "a{text-decoration:overline;text-decoration:underline dotted}";
10759+
expect(minifyForWith(css, T, { lowerUnsupported: false })).toBe(css);
10760+
});
10761+
});
10762+
1067510763
describe("CssSyntax minify — nesting the target cannot read", () => {
1067610764
it("writes a rule nested in another on its own", () => {
1067710765
expect(minifyFor("a{color:red;& b{color:blue}}", ["chrome 100"])).toBe(

0 commit comments

Comments
 (0)