Skip to content

Commit d5a9ea8

Browse files
alexander-akaitalexander-akait
committed
fix(javascript): read a comment's line break as terser's tokenizer does
terser steps over a comment's own characters before it makes the token, so a block comment holding a line break reads as having one before it; webpack's parse read only the gap ahead of it, and wrote `!/*!…` where terser puts the `!` on a line of its own. A parenthesis hands its break to the first comment inside it only where it holds no comment of its own, as terser's parser has it, and a block comment is read with its breaks spelled `\n`. The swc corpus differed over `projects/files` and `benches-full`, where each library's banner comment is followed by `(function (…) {`; all nine print byte for byte terser's again.
1 parent 5f281e9 commit d5a9ea8

2 files changed

Lines changed: 62 additions & 6 deletions

File tree

‎lib/javascript/syntax-printer.js‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14668,7 +14668,9 @@ const installEstree = (modules) => {
1466814668
: [...outer, ...inner.comments_before];
1466914669
inner.comments_before = before;
1467014670
open.comments_before = before;
14671-
if (before.length > 0) {
14671+
// Only a parenthesis holding no comment of its own hands its line break
14672+
// to the first comment inside it, as terser's parser has it.
14673+
if (outer.length === 0 && before.length > 0) {
1467214674
const [comment] = before;
1467314675
if (!comment.nlb) {
1467414676
comment.nlb = open.nlb;
@@ -16725,17 +16727,23 @@ const installFusedParse = (modules) => {
1672516727
nlb =
1672616728
code === 10 || code === 13 || code === 0x2028 || code === 0x2029;
1672716729
}
16730+
// A block comment is read with every line break in it spelled `\n`,
16731+
// which is what terser's tokenizer leaves for the printer to write.
16732+
const value = isBlock
16733+
? text.replace(/\r\n|[\r\u2028\u2029]/g, "\n")
16734+
: text;
16735+
// WHY: terser's tokenizer steps over a comment's own characters before
16736+
// it makes the token, so one holding a line break reads as having a
16737+
// break before it as well as breaking the line for what follows.
16738+
brokenLine = isBlock && value.includes("\n");
1672816739
read(
1672916740
starts.length,
1673016741
isBlock ? "comment2" : "comment1",
16731-
text,
16742+
value,
1673216743
start,
16733-
nlb
16744+
nlb || brokenLine
1673416745
);
1673516746
previousEnd = end;
16736-
// Reading it clears the break before it, and a comment holding one
16737-
// breaks the line for what follows.
16738-
brokenLine = isBlock && /[\n\r\u2028\u2029]/.test(text);
1673916747
},
1674016748
onToken: (/** @type {EXPECTED_ANY} */ token) => {
1674116749
let nlb = brokenLine;

‎test/unitCases/syntax-printer.unittest.js‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1680,6 +1680,54 @@ describe("syntax-printer", () => {
16801680
});
16811681
}
16821682

1683+
// Layouts where a comment decides where a line break is printed, which
1684+
// terser's tokenizer and its parentheses each have a rule for.
1685+
/** @type {[string, string][]} */
1686+
const COMMENT_BREAK_CASES = [
1687+
[
1688+
"a banner comment before a parenthesized call",
1689+
"/*!\n * banner\n */\n(function (w) { w.x = 1; })(this);"
1690+
],
1691+
[
1692+
"a banner comment read after an operator",
1693+
"!/*!\n * banner\n */\nfunction (g) { g.x = 1; }(this);"
1694+
],
1695+
[
1696+
"a comment of a parenthesis, on the line before it",
1697+
"var a = 1;/*! keep */\n(function () { a++; })();"
1698+
],
1699+
[
1700+
"two comments of a parenthesis, on the line before it",
1701+
"var a = 1;/*! one */ /*! two */\n(function () { a++; })();"
1702+
],
1703+
[
1704+
"a comment on the line its parenthesis opens",
1705+
"/*! a */(function () {})();"
1706+
],
1707+
[
1708+
"a comment inside a parenthesis holding none of its own",
1709+
"sink((/*! a */ function () {})());"
1710+
],
1711+
[
1712+
"a comment whose line breaks are carriage returns",
1713+
"/*!\r\n * banner\r\n */\r\n(function (w) { w.x = 1; })(this);\r\n"
1714+
],
1715+
[
1716+
"a comment broken by a line separator",
1717+
"/*!\u2028 * banner\u2029 */\n(function (w) { w.x = 1; })(this);"
1718+
]
1719+
];
1720+
for (const [name, source] of COMMENT_BREAK_CASES) {
1721+
it(`should break a line around a comment as terser does: ${name}`, async () => {
1722+
const { minify } = await load();
1723+
const reference = terserReference();
1724+
const options = { compress: false, mangle: false };
1725+
const ours = await minify(source, { ...options });
1726+
const theirs = await reference.minify(source, { ...options });
1727+
expect(ours.code).toBe(theirs.code);
1728+
});
1729+
}
1730+
16831731
// `wrap` and `enclose` are options terser reads but its types omit.
16841732
/** @type {[string, string, EXPECTED_OBJECT][]} */
16851733
const DEFINE_CASES = [

0 commit comments

Comments
 (0)