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 c60fd92787275..89434d1fd82e0 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, @@ -1254,7 +1257,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,44 +3167,97 @@ 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 commentWillEmitNewLine(node: CommentRange) { - return node.kind === SyntaxKind.SingleLineCommentTrivia || !!node.hasTrailingNewLine; + 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(); + } + 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(); + } + } + + // 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; + // 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 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 { @@ -3857,10 +3914,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 +3932,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 +4848,10 @@ 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); + emitTrailingCommentsOfPosition(commentRange.pos); } else { shouldEmitInterveningComments = mayEmitInterveningComments; diff --git a/tests/cases/compiler/jsxCommentDuplication.tsx b/tests/cases/compiler/jsxCommentDuplication.tsx new file mode 100644 index 0000000000000..6c2b42479056c --- /dev/null +++ b/tests/cases/compiler/jsxCommentDuplication.tsx @@ -0,0 +1,15 @@ +// @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 */; + +// 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