From fadc081db163130903539e81c2384233882b0436 Mon Sep 17 00:00:00 2001 From: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com> Date: Wed, 22 Nov 2023 19:52:23 +0800 Subject: [PATCH] fix: Unexpected duplication of comments (#16110) --- babel.config.js | 21 +++-- packages/babel-generator/src/buffer.ts | 3 - packages/babel-generator/src/printer.ts | 4 + .../output.js | 2 - packages/babel-generator/test/index.js | 20 +++++ .../test/fixtures/lazy-dep/unused/output.js | 1 + .../parameters/default-arguments/output.js | 1 - .../exports/declared-types/output.mjs | 1 + .../exports/export-type-star-from/output.mjs | 1 + packages/babel-traverse/src/path/comments.ts | 8 +- .../fixtures/comments/attachment/output.js | 4 +- packages/babel-traverse/test/removal.js | 81 +++++++++++++++++++ packages/babel-traverse/test/replacement.js | 14 ++-- 13 files changed, 136 insertions(+), 25 deletions(-) diff --git a/babel.config.js b/babel.config.js index 8232b1240be3..e7779d9e9425 100644 --- a/babel.config.js +++ b/babel.config.js @@ -1,12 +1,13 @@ "use strict"; -if ( - typeof it === "function" && +let jestSnapshot = false; +if (typeof it === "function") { // Jest loads the Babel config to parse file and update inline snapshots. // This is ok, as it's not loading the Babel config to test Babel itself. - !new Error().stack.includes("jest-snapshot") -) { - throw new Error("Monorepo root's babel.config.js loaded by a test."); + if (!new Error().stack.includes("jest-snapshot")) { + throw new Error("Monorepo root's babel.config.js loaded by a test."); + } + jestSnapshot = true; } const pathUtils = require("path"); @@ -304,6 +305,16 @@ module.exports = function (api) { ].filter(Boolean), }; + if (jestSnapshot) { + config.plugins = []; + config.presets = []; + config.overrides = []; + config.parserOpts = { + plugins: ["typescript"], + }; + config.sourceType = "unambiguous"; + } + return config; }; diff --git a/packages/babel-generator/src/buffer.ts b/packages/babel-generator/src/buffer.ts index 0519bb6a2ca0..c9e6ec65fedd 100644 --- a/packages/babel-generator/src/buffer.ts +++ b/packages/babel-generator/src/buffer.ts @@ -379,9 +379,6 @@ export default class Buffer { /** * check if current _last + queue ends with newline, return the character before newline - * - * @param {*} ch - * @memberof Buffer */ endsWithCharAndNewline(): number { const queue = this._queue; diff --git a/packages/babel-generator/src/printer.ts b/packages/babel-generator/src/printer.ts index 01cdf9e336d4..a02c59b379b9 100644 --- a/packages/babel-generator/src/printer.ts +++ b/packages/babel-generator/src/printer.ts @@ -824,6 +824,10 @@ class Printer { if (i < len - 1) separator?.(); if (opts.statement) { + if (!node.trailingComments?.length) { + this._lastCommentLine = 0; + } + if (i + 1 === len) { this.newline(1); } else { diff --git a/packages/babel-generator/test/fixtures/comments/variable-declarator-trailing-comment/output.js b/packages/babel-generator/test/fixtures/comments/variable-declarator-trailing-comment/output.js index 9347d91a23d5..fc9216b79a39 100644 --- a/packages/babel-generator/test/fixtures/comments/variable-declarator-trailing-comment/output.js +++ b/packages/babel-generator/test/fixtures/comments/variable-declarator-trailing-comment/output.js @@ -3,13 +3,11 @@ * This is trailing comment */ } - { var tt = 20; /* * This is trailing comment */ } - { { var t = 20; /* diff --git a/packages/babel-generator/test/index.js b/packages/babel-generator/test/index.js index bd788626b4a8..021f7de6887b 100644 --- a/packages/babel-generator/test/index.js +++ b/packages/babel-generator/test/index.js @@ -1492,6 +1492,26 @@ describe("programmatic generation", function () { val );`); }); + + it("multi-line leading comment after return 2", () => { + const ast = parse( + `return ( + /*new + line*/ val);`, + { + allowReturnOutsideFunction: true, + }, + ); + // Remove `parenthesized` + ast.program.body[0].argument.extra = null; + expect(generate(ast).code).toMatchInlineSnapshot(` + "return ( + /*new + line*/ + val + );" + `); + }); }); }); diff --git a/packages/babel-plugin-transform-modules-commonjs/test/fixtures/lazy-dep/unused/output.js b/packages/babel-plugin-transform-modules-commonjs/test/fixtures/lazy-dep/unused/output.js index a7b2c54a006a..09e3593034cc 100644 --- a/packages/babel-plugin-transform-modules-commonjs/test/fixtures/lazy-dep/unused/output.js +++ b/packages/babel-plugin-transform-modules-commonjs/test/fixtures/lazy-dep/unused/output.js @@ -23,4 +23,5 @@ _e().e; // The first import is unused, but we keep the require call // because of the second one + _g().h; diff --git a/packages/babel-plugin-transform-parameters/test/fixtures/parameters/default-arguments/output.js b/packages/babel-plugin-transform-parameters/test/fixtures/parameters/default-arguments/output.js index 2172aa8c3005..372e73a07071 100644 --- a/packages/babel-plugin-transform-parameters/test/fixtures/parameters/default-arguments/output.js +++ b/packages/babel-plugin-transform-parameters/test/fixtures/parameters/default-arguments/output.js @@ -4,5 +4,4 @@ function func() { } console.log(_arguments); // [1, 2, 3] } - func(1, 2, 3); diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/exports/declared-types/output.mjs b/packages/babel-plugin-transform-typescript/test/fixtures/exports/declared-types/output.mjs index dd3c3ff4de96..06a4cf92860c 100644 --- a/packages/babel-plugin-transform-typescript/test/fixtures/exports/declared-types/output.mjs +++ b/packages/babel-plugin-transform-typescript/test/fixtures/exports/declared-types/output.mjs @@ -23,5 +23,6 @@ function foo() {} export { BB2 as BB3, foo }; // only BB2->BB3 and foo // export an interface before declaration + // everything removed export { C2 as C4 }; // only C2->C4 diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-type-star-from/output.mjs b/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-type-star-from/output.mjs index c69e786c02dc..85ed612456c0 100644 --- a/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-type-star-from/output.mjs +++ b/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-type-star-from/output.mjs @@ -1,4 +1,5 @@ // Note: TSC doesn't support string module specifiers yet, // but it's easier for us to support them than not. + ; export {}; diff --git a/packages/babel-traverse/src/path/comments.ts b/packages/babel-traverse/src/path/comments.ts index e4c43b2e9bf1..4faff221974a 100644 --- a/packages/babel-traverse/src/path/comments.ts +++ b/packages/babel-traverse/src/path/comments.ts @@ -47,12 +47,10 @@ export function shareCommentsWithSiblings(this: NodePath) { } function removeIfExisting(list: T[], toRemove?: T[]): T[] { - if (!toRemove) return list; - let lastFoundIndex = -1; + if (!toRemove?.length) return list; + const set = new Set(toRemove); return list.filter(el => { - const i = toRemove.indexOf(el, lastFoundIndex); - if (i === -1) return true; - lastFoundIndex = i; + return !set.has(el); }); } diff --git a/packages/babel-traverse/test/fixtures/comments/attachment/output.js b/packages/babel-traverse/test/fixtures/comments/attachment/output.js index c54cdac3d2ef..b6cd5b85eabf 100644 --- a/packages/babel-traverse/test/fixtures/comments/attachment/output.js +++ b/packages/babel-traverse/test/fixtures/comments/attachment/output.js @@ -2,8 +2,7 @@ { hasPrev; /* top */ - /* left */ - /* right */ + /* left */ /* right */ /* bottom */ } { @@ -16,6 +15,7 @@ hasPrev; /* top */ /* left */ + /* right */ /* bottom */ hasNext; diff --git a/packages/babel-traverse/test/removal.js b/packages/babel-traverse/test/removal.js index e1b313e452c1..6f066bf73545 100644 --- a/packages/babel-traverse/test/removal.js +++ b/packages/babel-traverse/test/removal.js @@ -46,6 +46,87 @@ describe("removal", function () { expect(generate(ast).code).toBe(""); }); + it("leading comments", function () { + const ast = parse(` + // update-tsconfig-file + const a = 5; + // const updateTSConfig = require('../update-tsconfig') + // https://nextjs.org/docs/app/building-your-application/upgrading/from-vite + const getValue = (a) => a.value; + getValue(); +`); + traverse(ast, { + VariableDeclarator: function (path) { + path.remove(); + }, + }); + + expect(ast.program.body[0].leadingComments).toMatchInlineSnapshot(` + Array [ + Object { + "end": 28, + "loc": SourceLocation { + "end": Position { + "column": 27, + "index": 28, + "line": 2, + }, + "filename": undefined, + "identifierName": undefined, + "start": Position { + "column": 4, + "index": 5, + "line": 2, + }, + }, + "start": 5, + "type": "CommentLine", + "value": " update-tsconfig-file", + }, + Object { + "end": 105, + "loc": SourceLocation { + "end": Position { + "column": 59, + "index": 105, + "line": 4, + }, + "filename": undefined, + "identifierName": undefined, + "start": Position { + "column": 4, + "index": 50, + "line": 4, + }, + }, + "start": 50, + "type": "CommentLine", + "value": " const updateTSConfig = require('../update-tsconfig')", + }, + Object { + "end": 186, + "loc": SourceLocation { + "end": Position { + "column": 80, + "index": 186, + "line": 5, + }, + "filename": undefined, + "identifierName": undefined, + "start": Position { + "column": 4, + "index": 110, + "line": 5, + }, + }, + "start": 110, + "type": "CommentLine", + "value": " https://nextjs.org/docs/app/building-your-application/upgrading/from-vite", + }, + ] + `); + }); + describe("within an IfStatement", function () { it("does not make consequent null", function () { const rootPath = getPath("if (x) foo(); else bar();"); diff --git a/packages/babel-traverse/test/replacement.js b/packages/babel-traverse/test/replacement.js index 994be4679bd3..f6579e9e335e 100644 --- a/packages/babel-traverse/test/replacement.js +++ b/packages/babel-traverse/test/replacement.js @@ -342,13 +342,13 @@ describe("path/replacement", function () { const node = parseStmt("{ return }"); path.replaceExpressionWithStatements([undefinedNode, node]); expect(path.toString()).toMatchInlineSnapshot(` - "function () { - undefined; - { - return; - } - }()" - `); + "function () { + undefined; + { + return; + } + }()" + `); }); }); });