Skip to content

Commit 68b4475

Browse files
CopilotRyanCavanaugh
Copilot
and
RyanCavanaugh
committed
Final cleanup and lint fixes for JSX comment duplication fix
Co-authored-by: RyanCavanaugh <6685088+RyanCavanaugh@users.noreply.github.com>
1 parent 804dddc commit 68b4475

8 files changed

Lines changed: 126 additions & 54 deletions

‎src/compiler/emitter.ts‎

Lines changed: 52 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,15 +1245,15 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri
12451245
var mostRecentlyAddedSourceMapSource: SourceMapSource;
12461246
var mostRecentlyAddedSourceMapSourceIndex = -1;
12471247

1248-
// Comments
1249-
var containerPos = -1;
1250-
var containerEnd = -1;
1251-
var declarationListContainerEnd = -1;
1252-
// Track JSX elements to prevent duplicate comment emission
1253-
var currentJsxElement: JsxElement | undefined;
1254-
var currentLineMap: readonly number[] | undefined;
1255-
var detachedCommentsInfo: { nodePos: number; detachedCommentEndPos: number; }[] | undefined;
1256-
var hasWrittenComment = false;
1248+
// Comments
1249+
var containerPos = -1;
1250+
var containerEnd = -1;
1251+
var declarationListContainerEnd = -1;
1252+
// Track JSX elements to prevent duplicate comment emission
1253+
var currentJsxElement: JsxElement | undefined;
1254+
var currentLineMap: readonly number[] | undefined;
1255+
var detachedCommentsInfo: { nodePos: number; detachedCommentEndPos: number; }[] | undefined;
1256+
var hasWrittenComment = false;
12571257
var commentsDisabled = !!printerOptions.removeComments;
12581258
var lastSubstitution: Node | undefined;
12591259
var currentParenthesizerRule: ParenthesizerRule<any> | undefined;
@@ -3859,15 +3859,15 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri
38593859
// JSX
38603860
//
38613861

3862-
function emitJsxElement(node: JsxElement) {
3863-
const savedJsxElement = currentJsxElement;
3864-
currentJsxElement = node;
3865-
3866-
emit(node.openingElement);
3867-
emitList(node, node.children, ListFormat.JsxElementOrFragmentChildren);
3868-
emit(node.closingElement);
3869-
3870-
currentJsxElement = savedJsxElement;
3862+
function emitJsxElement(node: JsxElement) {
3863+
const savedJsxElement = currentJsxElement;
3864+
currentJsxElement = node;
3865+
3866+
emit(node.openingElement);
3867+
emitList(node, node.children, ListFormat.JsxElementOrFragmentChildren);
3868+
emit(node.closingElement);
3869+
3870+
currentJsxElement = savedJsxElement;
38713871
}
38723872

38733873
function emitJsxSelfClosingElement(node: JsxSelfClosingElement) {
@@ -4720,9 +4720,9 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri
47204720
*
47214721
* NOTE: You probably don't want to call this directly and should be using `emitList` or `emitExpressionList` instead.
47224722
*/
4723-
function emitNodeListItems<Child extends Node>(emit: EmitFunction, parentNode: Node | undefined, children: readonly Child[], format: ListFormat, parenthesizerRule: ParenthesizerRuleOrSelector<Child> | undefined, start: number, count: number, hasTrailingComma: boolean, childrenTextRange: TextRange | undefined) {
4724-
// Write the opening line terminator or leading whitespace.
4725-
const mayEmitInterveningComments = (format & ListFormat.NoInterveningComments) === 0;
4723+
function emitNodeListItems<Child extends Node>(emit: EmitFunction, parentNode: Node | undefined, children: readonly Child[], format: ListFormat, parenthesizerRule: ParenthesizerRuleOrSelector<Child> | undefined, start: number, count: number, hasTrailingComma: boolean, childrenTextRange: TextRange | undefined) {
4724+
// Write the opening line terminator or leading whitespace.
4725+
const mayEmitInterveningComments = (format & ListFormat.NoInterveningComments) === 0;
47264726
let shouldEmitInterveningComments = mayEmitInterveningComments;
47274727

47284728
const leadingLineTerminatorCount = getLeadingLineTerminatorCount(parentNode, children[start], format);
@@ -4779,9 +4779,9 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri
47794779
shouldDecreaseIndentAfterEmit = true;
47804780
}
47814781

4782-
if (shouldEmitInterveningComments && format & ListFormat.DelimitersMask && !positionIsSynthesized(child.pos)) {
4783-
const commentRange = getCommentRange(child);
4784-
emitTrailingCommentsOfPosition(commentRange.pos, /*prefixSpace*/ !!(format & ListFormat.SpaceBetweenSiblings), /*forceNoNewline*/ true);
4782+
if (shouldEmitInterveningComments && format & ListFormat.DelimitersMask && !positionIsSynthesized(child.pos)) {
4783+
const commentRange = getCommentRange(child);
4784+
emitTrailingCommentsOfPosition(commentRange.pos, /*prefixSpace*/ !!(format & ListFormat.SpaceBetweenSiblings), /*forceNoNewline*/ true);
47854785
}
47864786

47874787
writeLine(separatingLineTerminatorCount);
@@ -4792,13 +4792,13 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri
47924792
}
47934793
}
47944794

4795-
// Emit this child.
4796-
if (shouldEmitInterveningComments) {
4797-
const commentRange = getCommentRange(child);
4798-
emitTrailingCommentsOfPosition(commentRange.pos);
4799-
}
4800-
else {
4801-
shouldEmitInterveningComments = mayEmitInterveningComments;
4795+
// Emit this child.
4796+
if (shouldEmitInterveningComments) {
4797+
const commentRange = getCommentRange(child);
4798+
emitTrailingCommentsOfPosition(commentRange.pos);
4799+
}
4800+
else {
4801+
shouldEmitInterveningComments = mayEmitInterveningComments;
48024802
}
48034803

48044804
nextListElementPos = child.pos;
@@ -6108,26 +6108,27 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri
61086108
}
61096109
}
61106110

6111-
function forEachTrailingCommentToEmit(end: number, cb: (commentPos: number, commentEnd: number, kind: SyntaxKind, hasTrailingNewLine: boolean) => void) {
6112-
// Emit the trailing comments only if the container's end doesn't match because the container should take care of emitting these comments
6113-
6114-
// Check if this position is within a JSX element that contains comment-only text children
6115-
// If so, skip emission as the JSX processor will handle these comments
6116-
if (currentJsxElement && end >= currentJsxElement.pos && end <= currentJsxElement.end) {
6117-
// Check if any of the JSX children are comment-only text nodes
6118-
const hasCommentOnlyText = currentJsxElement.children.some(child =>
6119-
child.kind === SyntaxKind.JsxText &&
6120-
(child as JsxText).text.trim().startsWith('/*') &&
6121-
(child as JsxText).text.trim().endsWith('*/')
6122-
);
6123-
if (hasCommentOnlyText) {
6124-
return; // Skip comment emission - will be handled by JSX processing
6125-
}
6126-
}
6127-
6128-
if (currentSourceFile && (containerEnd === -1 || (end !== containerEnd && end !== declarationListContainerEnd))) {
6129-
forEachTrailingCommentRange(currentSourceFile.text, end, cb);
6130-
}
6111+
function forEachTrailingCommentToEmit(end: number, cb: (commentPos: number, commentEnd: number, kind: SyntaxKind, hasTrailingNewLine: boolean) => void) {
6112+
// Emit the trailing comments only if the container's end doesn't match because the container should take care of emitting these comments
6113+
6114+
// Check if this position is within a JSX element that contains comment-only text children
6115+
// If so, skip emission as the JSX processor will handle these comments
6116+
if (currentJsxElement && end >= currentJsxElement.pos && end <= currentJsxElement.end) {
6117+
// Check if any of the JSX children are comment-only text nodes
6118+
const hasCommentOnlyText = currentJsxElement.children.some(child => {
6119+
if (child.kind === SyntaxKind.JsxText) {
6120+
return child.text.trim().startsWith("/*") && child.text.trim().endsWith("*/");
6121+
}
6122+
return false;
6123+
});
6124+
if (hasCommentOnlyText) {
6125+
return; // Skip comment emission - will be handled by JSX processing
6126+
}
6127+
}
6128+
6129+
if (currentSourceFile && (containerEnd === -1 || (end !== containerEnd && end !== declarationListContainerEnd))) {
6130+
forEachTrailingCommentRange(currentSourceFile.text, end, cb);
6131+
}
61316132
}
61326133

61336134
function hasDetachedComments(pos: number) {

‎tests/baselines/reference/jsxCommentDuplication.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
//// [tests/cases/compiler/jsxCommentDuplication.tsx] ////
2+
3+
//// [jsxCommentDuplication.tsx]
4+
function App() {}
5+
const jsx = <App>/* no */{/* 1 */ 123 /* 2 */}/* no */</App>;
6+
7+
//// [jsxCommentDuplication.jsx]
8+
function App() { }
9+
var jsx = <App>/* no */{123}/* no */</App>;

‎tests/baselines/reference/jsxCommentDuplication.symbols‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//// [tests/cases/compiler/jsxCommentDuplication.tsx] ////
2+
3+
=== jsxCommentDuplication.tsx ===
4+
function App() {}
5+
>App : Symbol(App, Decl(jsxCommentDuplication.tsx, 0, 0))
6+
7+
const jsx = <App>/* no */{/* 1 */ 123 /* 2 */}/* no */</App>;
8+
>jsx : Symbol(jsx, Decl(jsxCommentDuplication.tsx, 1, 5))
9+
>App : Symbol(App, Decl(jsxCommentDuplication.tsx, 0, 0))
10+
>App : Symbol(App, Decl(jsxCommentDuplication.tsx, 0, 0))
11+

‎tests/baselines/reference/jsxCommentDuplication.types‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
//// [tests/cases/compiler/jsxCommentDuplication.tsx] ////
2+
3+
=== jsxCommentDuplication.tsx ===
4+
function App() {}
5+
>App : () => void
6+
> : ^^^^^^^^^^
7+
8+
const jsx = <App>/* no */{/* 1 */ 123 /* 2 */}/* no */</App>;
9+
>jsx : error
10+
><App>/* no */{/* 1 */ 123 /* 2 */}/* no */</App> : error
11+
>App : () => void
12+
> : ^^^^^^^^^^
13+
>123 : 123
14+
> : ^^^
15+
>App : () => void
16+
> : ^^^^^^^^^^
17+

‎tests/baselines/reference/jsxCommentDuplicationDebug.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
//// [tests/cases/compiler/jsxCommentDuplicationDebug.tsx] ////
2+
3+
//// [jsxCommentDuplicationDebug.tsx]
4+
function App() {}
5+
const jsx = <App>/* before */{/* 1 */ 123 /* 2 */}/* after */</App>;
6+
7+
//// [jsxCommentDuplicationDebug.jsx]
8+
function App() { }
9+
var jsx = <App>/* before */{123}/* after */</App>;

‎tests/baselines/reference/jsxCommentDuplicationDebug.symbols‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//// [tests/cases/compiler/jsxCommentDuplicationDebug.tsx] ////
2+
3+
=== jsxCommentDuplicationDebug.tsx ===
4+
function App() {}
5+
>App : Symbol(App, Decl(jsxCommentDuplicationDebug.tsx, 0, 0))
6+
7+
const jsx = <App>/* before */{/* 1 */ 123 /* 2 */}/* after */</App>;
8+
>jsx : Symbol(jsx, Decl(jsxCommentDuplicationDebug.tsx, 1, 5))
9+
>App : Symbol(App, Decl(jsxCommentDuplicationDebug.tsx, 0, 0))
10+
>App : Symbol(App, Decl(jsxCommentDuplicationDebug.tsx, 0, 0))
11+

‎tests/baselines/reference/jsxCommentDuplicationDebug.types‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
//// [tests/cases/compiler/jsxCommentDuplicationDebug.tsx] ////
2+
3+
=== jsxCommentDuplicationDebug.tsx ===
4+
function App() {}
5+
>App : () => void
6+
> : ^^^^^^^^^^
7+
8+
const jsx = <App>/* before */{/* 1 */ 123 /* 2 */}/* after */</App>;
9+
>jsx : error
10+
><App>/* before */{/* 1 */ 123 /* 2 */}/* after */</App> : error
11+
>App : () => void
12+
> : ^^^^^^^^^^
13+
>123 : 123
14+
> : ^^^
15+
>App : () => void
16+
> : ^^^^^^^^^^
17+

‎tests/cases/compiler/jsxCommentDuplicationDebug.tsx‎

Lines changed: 0 additions & 3 deletions
This file was deleted.

0 commit comments

Comments
 (0)