Skip to content

Fix JSX comment duplication in preserve mode #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions .github/copilot-questions.md
Original file line number Diff line number Diff line change
@@ -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.
171 changes: 117 additions & 54 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,12 @@ import {
isInJsonFile,
isJSDocLikeText,
isJsonSourceFile,
isJsxClosingElement,
isJsxNamespacedName,
isJsxOpeningElement,
isJsxClosingElement,
isJsxElement,
isJsxFragment,
isJsxNamespacedName,
isJsxOpeningElement,
isJsxText,
isKeyword,
isLet,
isLiteralExpression,
Expand Down Expand Up @@ -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<any> | undefined;
var currentParenthesizerRule: ParenthesizerRule<any> | undefined;
var isEmittingJsxChildren = false;
var { enter: enterComment, exit: exitComment } = performance.createTimerIf(extendedDiagnostics, "commentTime", "beforeComment", "afterComment");
var parenthesizer = factory.parenthesizer;
var typeArgumentParenthesizerRuleSelector: OrdinalParentheizerRuleSelector<TypeNode> = {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
15 changes: 15 additions & 0 deletions tests/cases/compiler/jsxCommentDuplication.tsx
Original file line number Diff line number Diff line change
@@ -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 = <div>/*pre*/{x}/*post*/</div>;

// Original issue repro
const jsx2 = <App>/* no */{/* 1 */ 123 /* 2 */}/* no */</App>;

// Additional edge cases based on maintainer's note about leading vs trailing behavior
const jsx3 = <div>/*leading*/{123}</div>;
const jsx4 = <div>{123}/*trailing*/</div>;