Skip to content

Commit c3ff0d4

Browse files
weswighamweswigham
authored
Under jsx: preserve, actually preserve expressions which contain only comments (microsoft#41757)
* Under jsx: preserve, actually preserve expressions which contain only comments * Even better best effort comment preservation in JSX comments
1 parent caebbe6 commit c3ff0d4

File tree

40 files changed

+1034
-32
lines changed

40 files changed

+1034
-32
lines changed

‎src/compiler/emitter.ts

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2864,7 +2864,8 @@ namespace ts {
28642864
}
28652865
pos = writeTokenText(token, writer, pos);
28662866
if (isSimilarNode && contextNode.end !== pos) {
2867-
emitTrailingCommentsOfPosition(pos, /*prefixSpace*/ true);
2867+
const isJsxExprContext = contextNode.kind === SyntaxKind.JsxExpression;
2868+
emitTrailingCommentsOfPosition(pos, /*prefixSpace*/ !isJsxExprContext, /*forceNoNewline*/ isJsxExprContext);
28682869
}
28692870
return pos;
28702871
}
@@ -3421,12 +3422,35 @@ namespace ts {
34213422
writePunctuation("}");
34223423
}
34233424

3425+
function hasTrailingCommentsAtPosition(pos: number) {
3426+
let result = false;
3427+
forEachTrailingCommentRange(currentSourceFile?.text || "", pos + 1, () => result = true);
3428+
return result;
3429+
}
3430+
3431+
function hasLeadingCommentsAtPosition(pos: number) {
3432+
let result = false;
3433+
forEachLeadingCommentRange(currentSourceFile?.text || "", pos + 1, () => result = true);
3434+
return result;
3435+
}
3436+
3437+
function hasCommentsAtPosition(pos: number) {
3438+
return hasTrailingCommentsAtPosition(pos) || hasLeadingCommentsAtPosition(pos);
3439+
}
3440+
34243441
function emitJsxExpression(node: JsxExpression) {
3425-
if (node.expression) {
3426-
writePunctuation("{");
3442+
if (node.expression || (!commentsDisabled && !nodeIsSynthesized(node) && hasCommentsAtPosition(node.pos))) { // preserve empty expressions if they contain comments!
3443+
const isMultiline = currentSourceFile && !nodeIsSynthesized(node) && getLineAndCharacterOfPosition(currentSourceFile, node.pos).line !== getLineAndCharacterOfPosition(currentSourceFile, node.end).line;
3444+
if (isMultiline) {
3445+
writer.increaseIndent();
3446+
}
3447+
const end = emitTokenWithComment(SyntaxKind.OpenBraceToken, node.pos, writePunctuation, node);
34273448
emit(node.dotDotDotToken);
34283449
emitExpression(node.expression);
3429-
writePunctuation("}");
3450+
emitTokenWithComment(SyntaxKind.CloseBraceToken, node.expression?.end || end, writePunctuation, node);
3451+
if (isMultiline) {
3452+
writer.decreaseIndent();
3453+
}
34303454
}
34313455
}
34323456

@@ -5274,15 +5298,27 @@ namespace ts {
52745298
}
52755299
}
52765300

5277-
function emitTrailingCommentsOfPosition(pos: number, prefixSpace?: boolean) {
5301+
function emitTrailingCommentsOfPosition(pos: number, prefixSpace?: boolean, forceNoNewline?: boolean) {
52785302
if (commentsDisabled) {
52795303
return;
52805304
}
52815305
enterComment();
5282-
forEachTrailingCommentToEmit(pos, prefixSpace ? emitTrailingComment : emitTrailingCommentOfPosition);
5306+
forEachTrailingCommentToEmit(pos, prefixSpace ? emitTrailingComment : forceNoNewline ? emitTrailingCommentOfPositionNoNewline : emitTrailingCommentOfPosition);
52835307
exitComment();
52845308
}
52855309

5310+
function emitTrailingCommentOfPositionNoNewline(commentPos: number, commentEnd: number, kind: SyntaxKind) {
5311+
// trailing comments of a position are emitted at /*trailing comment1 */space/*trailing comment*/space
5312+
5313+
emitPos(commentPos);
5314+
writeCommentRange(currentSourceFile!.text, getCurrentLineMap(), writer, commentPos, commentEnd, newLine);
5315+
emitPos(commentEnd);
5316+
5317+
if (kind === SyntaxKind.SingleLineCommentTrivia) {
5318+
writer.writeLine(); // still write a newline for single-line comments, so closing tokens aren't written on the same line
5319+
}
5320+
}
5321+
52865322
function emitTrailingCommentOfPosition(commentPos: number, commentEnd: number, _kind: SyntaxKind, hasTrailingNewLine: boolean) {
52875323
// trailing comments of a position are emitted at /*trailing comment1 */space/*trailing comment*/space
52885324

‎src/compiler/transformers/module/module.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1790,7 +1790,7 @@ namespace ts {
17901790
const name = importDeclaration.propertyName || importDeclaration.name;
17911791
return setTextRange(
17921792
factory.createPropertyAccessExpression(
1793-
factory.getGeneratedNameForNode(importDeclaration.parent.parent.parent),
1793+
factory.getGeneratedNameForNode(importDeclaration.parent?.parent?.parent || importDeclaration),
17941794
factory.cloneNode(name)
17951795
),
17961796
/*location*/ node

‎src/compiler/transformers/module/system.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1678,7 +1678,7 @@ namespace ts {
16781678
factory.createPropertyAssignment(
16791679
factory.cloneNode(name),
16801680
factory.createPropertyAccessExpression(
1681-
factory.getGeneratedNameForNode(importDeclaration.parent.parent.parent),
1681+
factory.getGeneratedNameForNode(importDeclaration.parent?.parent?.parent || importDeclaration),
16821682
factory.cloneNode(importDeclaration.propertyName || importDeclaration.name)
16831683
),
16841684
),
@@ -1747,7 +1747,7 @@ namespace ts {
17471747
else if (isImportSpecifier(importDeclaration)) {
17481748
return setTextRange(
17491749
factory.createPropertyAccessExpression(
1750-
factory.getGeneratedNameForNode(importDeclaration.parent.parent.parent),
1750+
factory.getGeneratedNameForNode(importDeclaration.parent?.parent?.parent || importDeclaration),
17511751
factory.cloneNode(importDeclaration.propertyName || importDeclaration.name)
17521752
),
17531753
/*location*/ node

‎tests/baselines/reference/commentEmittingInPreserveJsx1.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,19 +39,21 @@ var React = require("react");
3939
</div>;
4040
<div>
4141
// Not Comment
42-
42+
{
43+
//Comment just Fine
44+
}
4345
// Another not Comment
4446
</div>;
4547
<div>
4648
// Not Comment
4749
{
48-
//Comment just Fine
49-
"Hi"}
50+
//Comment just Fine
51+
"Hi"}
5052
// Another not Comment
5153
</div>;
5254
<div>
5355
/* Not Comment */
5456
{
55-
//Comment just Fine
56-
"Hi"}
57+
//Comment just Fine
58+
"Hi"}
5759
</div>;

‎tests/baselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=commonjs).js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
//// [commentsOnJSXExpressionsArePreserved.tsx]
2+
// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
3+
namespace JSX {}
4+
class Component {
5+
render() {
6+
return <div>
7+
{/* missing */}
8+
{null/* preserved */}
9+
{
10+
// ??? 1
11+
}
12+
{ // ??? 2
13+
}
14+
{// ??? 3
15+
}
16+
{
17+
// ??? 4
18+
/* ??? 5 */}
19+
</div>;
20+
}
21+
}
22+
23+
//// [commentsOnJSXExpressionsArePreserved.jsx]
24+
var Component = /** @class */ (function () {
25+
function Component() {
26+
}
27+
Component.prototype.render = function () {
28+
return <div>
29+
{/* missing */}
30+
{null /* preserved */}
31+
{
32+
// ??? 1
33+
}
34+
{// ??? 2
35+
}
36+
{// ??? 3
37+
}
38+
{
39+
// ??? 4
40+
/* ??? 5 */ }
41+
</div>;
42+
};
43+
return Component;
44+
}());

‎tests/baselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=commonjs).symbols

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
=== tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx ===
2+
// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
3+
namespace JSX {}
4+
>JSX : Symbol(JSX, Decl(commentsOnJSXExpressionsArePreserved.tsx, 0, 0))
5+
6+
class Component {
7+
>Component : Symbol(Component, Decl(commentsOnJSXExpressionsArePreserved.tsx, 1, 16))
8+
9+
render() {
10+
>render : Symbol(Component.render, Decl(commentsOnJSXExpressionsArePreserved.tsx, 2, 17))
11+
12+
return <div>
13+
{/* missing */}
14+
{null/* preserved */}
15+
{
16+
// ??? 1
17+
}
18+
{ // ??? 2
19+
}
20+
{// ??? 3
21+
}
22+
{
23+
// ??? 4
24+
/* ??? 5 */}
25+
</div>;
26+
}
27+
}

‎tests/baselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=commonjs).types

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
=== tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx ===
2+
// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
3+
namespace JSX {}
4+
class Component {
5+
>Component : Component
6+
7+
render() {
8+
>render : () => any
9+
10+
return <div>
11+
><div> {/* missing */} {null/* preserved */} { // ??? 1 } { // ??? 2 } {// ??? 3 } { // ??? 4 /* ??? 5 */} </div> : error
12+
>div : any
13+
14+
{/* missing */}
15+
{null/* preserved */}
16+
>null : null
17+
{
18+
// ??? 1
19+
}
20+
{ // ??? 2
21+
}
22+
{// ??? 3
23+
}
24+
{
25+
// ??? 4
26+
/* ??? 5 */}
27+
</div>;
28+
>div : any
29+
}
30+
}

‎tests/baselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=system).js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
//// [commentsOnJSXExpressionsArePreserved.tsx]
2+
// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
3+
namespace JSX {}
4+
class Component {
5+
render() {
6+
return <div>
7+
{/* missing */}
8+
{null/* preserved */}
9+
{
10+
// ??? 1
11+
}
12+
{ // ??? 2
13+
}
14+
{// ??? 3
15+
}
16+
{
17+
// ??? 4
18+
/* ??? 5 */}
19+
</div>;
20+
}
21+
}
22+
23+
//// [commentsOnJSXExpressionsArePreserved.jsx]
24+
var Component = /** @class */ (function () {
25+
function Component() {
26+
}
27+
Component.prototype.render = function () {
28+
return <div>
29+
{/* missing */}
30+
{null /* preserved */}
31+
{
32+
// ??? 1
33+
}
34+
{// ??? 2
35+
}
36+
{// ??? 3
37+
}
38+
{
39+
// ??? 4
40+
/* ??? 5 */ }
41+
</div>;
42+
};
43+
return Component;
44+
}());

‎tests/baselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=system).symbols

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
=== tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx ===
2+
// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
3+
namespace JSX {}
4+
>JSX : Symbol(JSX, Decl(commentsOnJSXExpressionsArePreserved.tsx, 0, 0))
5+
6+
class Component {
7+
>Component : Symbol(Component, Decl(commentsOnJSXExpressionsArePreserved.tsx, 1, 16))
8+
9+
render() {
10+
>render : Symbol(Component.render, Decl(commentsOnJSXExpressionsArePreserved.tsx, 2, 17))
11+
12+
return <div>
13+
{/* missing */}
14+
{null/* preserved */}
15+
{
16+
// ??? 1
17+
}
18+
{ // ??? 2
19+
}
20+
{// ??? 3
21+
}
22+
{
23+
// ??? 4
24+
/* ??? 5 */}
25+
</div>;
26+
}
27+
}

‎tests/baselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=system).types

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
=== tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx ===
2+
// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
3+
namespace JSX {}
4+
class Component {
5+
>Component : Component
6+
7+
render() {
8+
>render : () => any
9+
10+
return <div>
11+
><div> {/* missing */} {null/* preserved */} { // ??? 1 } { // ??? 2 } {// ??? 3 } { // ??? 4 /* ??? 5 */} </div> : error
12+
>div : any
13+
14+
{/* missing */}
15+
{null/* preserved */}
16+
>null : null
17+
{
18+
// ??? 1
19+
}
20+
{ // ??? 2
21+
}
22+
{// ??? 3
23+
}
24+
{
25+
// ??? 4
26+
/* ??? 5 */}
27+
</div>;
28+
>div : any
29+
}
30+
}

‎tests/baselines/reference/commentsOnJSXExpressionsArePreserved(jsx=react,module=commonjs).errors.txt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx(5,17): error TS2304: Cannot find name 'React'.
2+
3+
4+
==== tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx (1 errors) ====
5+
// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
6+
namespace JSX {}
7+
class Component {
8+
render() {
9+
return <div>
10+
~~~
11+
!!! error TS2304: Cannot find name 'React'.
12+
{/* missing */}
13+
{null/* preserved */}
14+
{
15+
// ??? 1
16+
}
17+
{ // ??? 2
18+
}
19+
{// ??? 3
20+
}
21+
{
22+
// ??? 4
23+
/* ??? 5 */}
24+
</div>;
25+
}
26+
}

0 commit comments

Comments
 (0)