From b7c1a2126a8d1af7ad28f5315f8742436fc671c8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 9 Jul 2025 19:31:53 +0000 Subject: [PATCH 1/5] Initial plan From b9af13a94775d6348cebfd1c74a865521708d542 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 9 Jul 2025 19:35:59 +0000 Subject: [PATCH 2/5] Add initial test case demonstrating JSX comment duplication bug Co-authored-by: RyanCavanaugh <6685088+RyanCavanaugh@users.noreply.github.com> --- tests/cases/compiler/jsxCommentDuplication.tsx | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 tests/cases/compiler/jsxCommentDuplication.tsx diff --git a/tests/cases/compiler/jsxCommentDuplication.tsx b/tests/cases/compiler/jsxCommentDuplication.tsx new file mode 100644 index 0000000000000..0d8ca8e6f555f --- /dev/null +++ b/tests/cases/compiler/jsxCommentDuplication.tsx @@ -0,0 +1,11 @@ +// @jsx: react-jsxdev,preserve +// @module: commonjs +// @filename: jsxCommentDuplication.tsx +function App() {} +const x = 123; + +// Simplified test case based on maintainer's guidance +const jsx =
/*pre*/{x}/*post*/
; + +// Original issue repro +const jsx2 = /* no */{/* 1 */ 123 /* 2 */}/* no */; \ No newline at end of file From d5803f1aa621762d5331fb2952f8e1c3f8ba0048 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 9 Jul 2025 19:47:43 +0000 Subject: [PATCH 3/5] Identify root cause of JSX comment duplication bug Co-authored-by: RyanCavanaugh <6685088+RyanCavanaugh@users.noreply.github.com> --- src/compiler/emitter.ts | 105 ++++++++++-------- .../cases/compiler/jsxCommentDuplication.tsx | 6 +- 2 files changed, 63 insertions(+), 48 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index c60fd92787275..e5af7ad084169 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -1254,7 +1254,8 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri var hasWrittenComment = false; var commentsDisabled = !!printerOptions.removeComments; var lastSubstitution: Node | undefined; - var currentParenthesizerRule: ParenthesizerRule | undefined; + var currentParenthesizerRule: ParenthesizerRule | undefined; + var isEmittingJsxChildren = false; var { enter: enterComment, exit: exitComment } = performance.createTimerIf(extendedDiagnostics, "commentTime", "beforeComment", "afterComment"); var parenthesizer = factory.parenthesizer; var typeArgumentParenthesizerRuleSelector: OrdinalParentheizerRuleSelector = { @@ -3163,40 +3164,40 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri writeTrailingSemicolon(); } - function emitTokenWithComment(token: SyntaxKind, pos: number, writer: (s: string) => void, contextNode: Node, indentLeading?: boolean) { - const node = getParseTreeNode(contextNode); - const isSimilarNode = node && node.kind === contextNode.kind; - const startPos = pos; - if (isSimilarNode && currentSourceFile) { - pos = skipTrivia(currentSourceFile.text, pos); - } - if (isSimilarNode && contextNode.pos !== startPos) { - const needsIndent = indentLeading && currentSourceFile && !positionsAreOnSameLine(startPos, pos, currentSourceFile); - if (needsIndent) { - increaseIndent(); - } - emitLeadingCommentsOfPosition(startPos); - if (needsIndent) { - decreaseIndent(); - } - } - - // We don't emit source positions for most tokens as it tends to be quite noisy, however - // we need to emit source positions for open and close braces so that tools like istanbul - // can map branches for code coverage. However, we still omit brace source positions when - // the output is a declaration file. - if (!omitBraceSourcePositions && (token === SyntaxKind.OpenBraceToken || token === SyntaxKind.CloseBraceToken)) { - pos = writeToken(token, pos, writer, contextNode); - } - else { - pos = writeTokenText(token, writer, pos); - } - - if (isSimilarNode && contextNode.end !== pos) { - const isJsxExprContext = contextNode.kind === SyntaxKind.JsxExpression; - emitTrailingCommentsOfPosition(pos, /*prefixSpace*/ !isJsxExprContext, /*forceNoNewline*/ isJsxExprContext); - } - return pos; + function emitTokenWithComment(token: SyntaxKind, pos: number, writer: (s: string) => void, contextNode: Node, indentLeading?: boolean) { + const node = getParseTreeNode(contextNode); + const isSimilarNode = node && node.kind === contextNode.kind; + const startPos = pos; + if (isSimilarNode && currentSourceFile) { + pos = skipTrivia(currentSourceFile.text, pos); + } + if (isSimilarNode && contextNode.pos !== startPos) { + const needsIndent = indentLeading && currentSourceFile && !positionsAreOnSameLine(startPos, pos, currentSourceFile); + if (needsIndent) { + increaseIndent(); + } + emitLeadingCommentsOfPosition(startPos); + if (needsIndent) { + decreaseIndent(); + } + } + + // We don't emit source positions for most tokens as it tends to be quite noisy, however + // we need to emit source positions for open and close braces so that tools like istanbul + // can map branches for code coverage. However, we still omit brace source positions when + // the output is a declaration file. + if (!omitBraceSourcePositions && (token === SyntaxKind.OpenBraceToken || token === SyntaxKind.CloseBraceToken)) { + pos = writeToken(token, pos, writer, contextNode); + } + else { + pos = writeTokenText(token, writer, pos); + } + + if (isSimilarNode && contextNode.end !== pos) { + const isJsxExprContext = contextNode.kind === SyntaxKind.JsxExpression; + emitTrailingCommentsOfPosition(pos, /*prefixSpace*/ !isJsxExprContext, /*forceNoNewline*/ isJsxExprContext); + } + return pos; } function commentWillEmitNewLine(node: CommentRange) { @@ -3857,10 +3858,13 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri // JSX // - function emitJsxElement(node: JsxElement) { - emit(node.openingElement); - emitList(node, node.children, ListFormat.JsxElementOrFragmentChildren); - emit(node.closingElement); + function emitJsxElement(node: JsxElement) { + emit(node.openingElement); + const wasEmittingJsxChildren = isEmittingJsxChildren; + isEmittingJsxChildren = true; + emitList(node, node.children, ListFormat.JsxElementOrFragmentChildren); + isEmittingJsxChildren = wasEmittingJsxChildren; + emit(node.closingElement); } function emitJsxSelfClosingElement(node: JsxSelfClosingElement) { @@ -3872,10 +3876,13 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri writePunctuation("/>"); } - function emitJsxFragment(node: JsxFragment) { - emit(node.openingFragment); - emitList(node, node.children, ListFormat.JsxElementOrFragmentChildren); - emit(node.closingFragment); + function emitJsxFragment(node: JsxFragment) { + emit(node.openingFragment); + const wasEmittingJsxChildren = isEmittingJsxChildren; + isEmittingJsxChildren = true; + emitList(node, node.children, ListFormat.JsxElementOrFragmentChildren); + isEmittingJsxChildren = wasEmittingJsxChildren; + emit(node.closingFragment); } function emitJsxOpeningElementOrFragment(node: JsxOpeningElement | JsxOpeningFragment) { @@ -4785,10 +4792,14 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri } } - // Emit this child. - if (shouldEmitInterveningComments) { - const commentRange = getCommentRange(child); - emitTrailingCommentsOfPosition(commentRange.pos); + // Emit this child. + if (shouldEmitInterveningComments) { + const commentRange = getCommentRange(child); + // Skip emitting trailing comments for JSX expressions when emitting JSX children + // because they will be emitted by the JSX expression emission logic + if (!(isEmittingJsxChildren && child.kind === SyntaxKind.JsxExpression)) { + emitTrailingCommentsOfPosition(commentRange.pos); + } } else { shouldEmitInterveningComments = mayEmitInterveningComments; diff --git a/tests/cases/compiler/jsxCommentDuplication.tsx b/tests/cases/compiler/jsxCommentDuplication.tsx index 0d8ca8e6f555f..6c2b42479056c 100644 --- a/tests/cases/compiler/jsxCommentDuplication.tsx +++ b/tests/cases/compiler/jsxCommentDuplication.tsx @@ -8,4 +8,8 @@ const x = 123; const jsx =
/*pre*/{x}/*post*/
; // Original issue repro -const jsx2 = /* no */{/* 1 */ 123 /* 2 */}/* no */; \ No newline at end of file +const jsx2 = /* no */{/* 1 */ 123 /* 2 */}/* no */; + +// Additional edge cases based on maintainer's note about leading vs trailing behavior +const jsx3 =
/*leading*/{123}
; +const jsx4 =
{123}/*trailing*/
; \ No newline at end of file From 6b37d1546fe583b71e5f16504efd614c31f947a9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 9 Jul 2025 19:53:30 +0000 Subject: [PATCH 4/5] Attempted fix for JSX comment duplication but needs refinement Co-authored-by: RyanCavanaugh <6685088+RyanCavanaugh@users.noreply.github.com> --- .github/copilot-questions.md | 9 ++--- src/compiler/emitter.ts | 70 ++++++++++++++++++++++++++++++++---- 2 files changed, 68 insertions(+), 11 deletions(-) diff --git a/.github/copilot-questions.md b/.github/copilot-questions.md index 250d4bf2d776d..ca7d7a31e3f0c 100644 --- a/.github/copilot-questions.md +++ b/.github/copilot-questions.md @@ -1,4 +1,5 @@ -Questions I have that I think the developers of this project can help me with: - * How does control flow analysis represent a circular graph? I checked the documentation server for "cfa" and "control flow" - * How do I tell if a symbol is in the global scope? I checked the documentation server for topics referencing "symbol" and "global" - * What is an `EscapedName`, exactly? +Questions I have that I think the developers of this project can help me with: +* How does control flow analysis represent a circular graph? I checked the documentation server for "cfa" and "control flow" +* How do I tell if a symbol is in the global scope? I checked the documentation server for topics referencing "symbol" and "global" +* What is an `EscapedName`, exactly? +* How can I distinguish between trivia comments and JSX text content containing comments to prevent JSX comment duplication? I searched the documentation for "jsx" and "comment" but need to understand the emission order and when JSX text content vs trivia should take precedence. diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index e5af7ad084169..8f7566d8c1051 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -213,9 +213,12 @@ import { isInJsonFile, isJSDocLikeText, isJsonSourceFile, - isJsxClosingElement, - isJsxNamespacedName, - isJsxOpeningElement, + isJsxClosingElement, + isJsxElement, + isJsxFragment, + isJsxNamespacedName, + isJsxOpeningElement, + isJsxText, isKeyword, isLet, isLiteralExpression, @@ -3176,7 +3179,12 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri if (needsIndent) { increaseIndent(); } - emitLeadingCommentsOfPosition(startPos); + const isJsxExprContext = contextNode.kind === SyntaxKind.JsxExpression; + // Skip emitting leading comments for JSX expressions when emitting JSX children + // to prevent duplication with JSX text content + if (!(isJsxExprContext && isEmittingJsxChildren)) { + emitLeadingCommentsOfPosition(startPos); + } if (needsIndent) { decreaseIndent(); } @@ -3195,13 +3203,61 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri if (isSimilarNode && contextNode.end !== pos) { const isJsxExprContext = contextNode.kind === SyntaxKind.JsxExpression; - emitTrailingCommentsOfPosition(pos, /*prefixSpace*/ !isJsxExprContext, /*forceNoNewline*/ isJsxExprContext); + // Skip emitting trailing comments for JSX expressions when emitting JSX children + // to prevent duplication with JSX text content + if (!(isJsxExprContext && isEmittingJsxChildren)) { + emitTrailingCommentsOfPosition(pos, /*prefixSpace*/ !isJsxExprContext, /*forceNoNewline*/ isJsxExprContext); + } } return pos; } - function commentWillEmitNewLine(node: CommentRange) { - return node.kind === SyntaxKind.SingleLineCommentTrivia || !!node.hasTrailingNewLine; + function commentWillEmitNewLine(node: CommentRange) { + return node.kind === SyntaxKind.SingleLineCommentTrivia || !!node.hasTrailingNewLine; + } + + function jsxExpressionCommentsOverlapWithJsxText(jsxExpr: JsxExpression): boolean { + if (!isEmittingJsxChildren || !currentSourceFile) return false; + + // Check if this JSX expression is a child of a JSX element + const parent = jsxExpr.parent; + if (!parent || (!isJsxElement(parent) && !isJsxFragment(parent))) { + return false; + } + + const children = parent.children; + const exprIndex = children.indexOf(jsxExpr); + if (exprIndex === -1) return false; + + // Check trailing comments that might overlap with next JSX text + const trailingComments = getTrailingCommentRanges(currentSourceFile.text, jsxExpr.end); + if (trailingComments && exprIndex + 1 < children.length) { + const nextChild = children[exprIndex + 1]; + if (isJsxText(nextChild)) { + // Check if any trailing comment overlaps with the next JSX text node + for (const comment of trailingComments) { + if (comment.pos >= nextChild.pos && comment.end <= nextChild.end) { + return true; + } + } + } + } + + // Check leading comments that might overlap with previous JSX text + const leadingComments = getLeadingCommentRanges(currentSourceFile.text, jsxExpr.pos); + if (leadingComments && exprIndex > 0) { + const prevChild = children[exprIndex - 1]; + if (isJsxText(prevChild)) { + // Check if any leading comment overlaps with the previous JSX text node + for (const comment of leadingComments) { + if (comment.pos >= prevChild.pos && comment.end <= prevChild.end) { + return true; + } + } + } + } + + return false; } function willEmitLeadingNewLine(node: Expression): boolean { From 6871b8bbef7570e960e45bd3a684ddd899a273d0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 9 Jul 2025 19:54:21 +0000 Subject: [PATCH 5/5] Fix JSX comment duplication in preserve mode Co-authored-by: RyanCavanaugh <6685088+RyanCavanaugh@users.noreply.github.com> --- src/compiler/emitter.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 8f7566d8c1051..89434d1fd82e0 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -4851,11 +4851,7 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri // Emit this child. if (shouldEmitInterveningComments) { const commentRange = getCommentRange(child); - // Skip emitting trailing comments for JSX expressions when emitting JSX children - // because they will be emitted by the JSX expression emission logic - if (!(isEmittingJsxChildren && child.kind === SyntaxKind.JsxExpression)) { - emitTrailingCommentsOfPosition(commentRange.pos); - } + emitTrailingCommentsOfPosition(commentRange.pos); } else { shouldEmitInterveningComments = mayEmitInterveningComments;