Skip to content

Commit

Permalink
fix: Unexpected duplication of comments (#16110)
Browse files Browse the repository at this point in the history
  • Loading branch information
liuxingbaoyu committed Nov 22, 2023
1 parent 099af39 commit fadc081
Show file tree
Hide file tree
Showing 13 changed files with 136 additions and 25 deletions.
21 changes: 16 additions & 5 deletions 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");
Expand Down Expand Up @@ -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;
};

Expand Down
3 changes: 0 additions & 3 deletions packages/babel-generator/src/buffer.ts
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions packages/babel-generator/src/printer.ts
Expand Up @@ -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 {
Expand Down
Expand Up @@ -3,13 +3,11 @@
* This is trailing comment
*/
}

{
var tt = 20; /*
* This is trailing comment
*/
}

{
{
var t = 20; /*
Expand Down
20 changes: 20 additions & 0 deletions packages/babel-generator/test/index.js
Expand Up @@ -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
);"
`);
});
});
});

Expand Down
Expand Up @@ -23,4 +23,5 @@ _e().e;

// The first import is unused, but we keep the require call
// because of the second one

_g().h;
Expand Up @@ -4,5 +4,4 @@ function func() {
}
console.log(_arguments); // [1, 2, 3]
}

func(1, 2, 3);
Expand Up @@ -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
@@ -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 {};
8 changes: 3 additions & 5 deletions packages/babel-traverse/src/path/comments.ts
Expand Up @@ -47,12 +47,10 @@ export function shareCommentsWithSiblings(this: NodePath) {
}

function removeIfExisting<T>(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);
});
}

Expand Down
Expand Up @@ -2,8 +2,7 @@
{
hasPrev;
/* top */
/* left */
/* right */
/* left */ /* right */
/* bottom */
}
{
Expand All @@ -16,6 +15,7 @@
hasPrev;
/* top */
/* left */

/* right */
/* bottom */
hasNext;
Expand Down
81 changes: 81 additions & 0 deletions packages/babel-traverse/test/removal.js
Expand Up @@ -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();");
Expand Down
14 changes: 7 additions & 7 deletions packages/babel-traverse/test/replacement.js
Expand Up @@ -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;
}
}()"
`);
});
});
});
Expand Down

0 comments on commit fadc081

Please sign in to comment.