Under jsx: preserve, actually preserve expressions which contain only…

… comments (#41757) * Under jsx: preserve, actually preserve expressions which contain only comments * Even better best effort comment preservation in JSX comments
microsoft · Dec 18, 2020 · c3ff0d4 · c3ff0d4
1 parent caebbe6
commit c3ff0d4
Show file tree

Hide file tree

Showing 40 changed files with 1,034 additions and 32 deletions.
diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts
@@ -2864,7 +2864,8 @@ namespace ts {
             }
             pos = writeTokenText(token, writer, pos);
             if (isSimilarNode && contextNode.end !== pos) {
-                emitTrailingCommentsOfPosition(pos, /*prefixSpace*/ true);
+                const isJsxExprContext = contextNode.kind === SyntaxKind.JsxExpression;
+                emitTrailingCommentsOfPosition(pos, /*prefixSpace*/ !isJsxExprContext, /*forceNoNewline*/ isJsxExprContext);
             }
             return pos;
         }
@@ -3421,12 +3422,35 @@ namespace ts {
             writePunctuation("}");
         }
 
+        function hasTrailingCommentsAtPosition(pos: number) {
+            let result = false;
+            forEachTrailingCommentRange(currentSourceFile?.text || "", pos + 1, () => result = true);
+            return result;
+        }
+
+        function hasLeadingCommentsAtPosition(pos: number) {
+            let result = false;
+            forEachLeadingCommentRange(currentSourceFile?.text || "", pos + 1, () => result = true);
+            return result;
+        }
+
+        function hasCommentsAtPosition(pos: number) {
+            return hasTrailingCommentsAtPosition(pos) || hasLeadingCommentsAtPosition(pos);
+        }
+
         function emitJsxExpression(node: JsxExpression) {
-            if (node.expression) {
-                writePunctuation("{");
+            if (node.expression || (!commentsDisabled && !nodeIsSynthesized(node) && hasCommentsAtPosition(node.pos))) { // preserve empty expressions if they contain comments!
+                const isMultiline = currentSourceFile && !nodeIsSynthesized(node) && getLineAndCharacterOfPosition(currentSourceFile, node.pos).line !== getLineAndCharacterOfPosition(currentSourceFile, node.end).line;
+                if (isMultiline) {
+                    writer.increaseIndent();
+                }
+                const end = emitTokenWithComment(SyntaxKind.OpenBraceToken, node.pos, writePunctuation, node);
                 emit(node.dotDotDotToken);
                 emitExpression(node.expression);
-                writePunctuation("}");
+                emitTokenWithComment(SyntaxKind.CloseBraceToken, node.expression?.end || end, writePunctuation, node);
+                if (isMultiline) {
+                    writer.decreaseIndent();
+                }
             }
         }
 
@@ -5274,15 +5298,27 @@ namespace ts {
             }
         }
 
-        function emitTrailingCommentsOfPosition(pos: number, prefixSpace?: boolean) {
+        function emitTrailingCommentsOfPosition(pos: number, prefixSpace?: boolean, forceNoNewline?: boolean) {
             if (commentsDisabled) {
                 return;
             }
             enterComment();
-            forEachTrailingCommentToEmit(pos, prefixSpace ? emitTrailingComment : emitTrailingCommentOfPosition);
+            forEachTrailingCommentToEmit(pos, prefixSpace ? emitTrailingComment : forceNoNewline ? emitTrailingCommentOfPositionNoNewline : emitTrailingCommentOfPosition);
             exitComment();
         }
 
+        function emitTrailingCommentOfPositionNoNewline(commentPos: number, commentEnd: number, kind: SyntaxKind) {
+            // trailing comments of a position are emitted at /*trailing comment1 */space/*trailing comment*/space
+
+            emitPos(commentPos);
+            writeCommentRange(currentSourceFile!.text, getCurrentLineMap(), writer, commentPos, commentEnd, newLine);
+            emitPos(commentEnd);
+
+            if (kind === SyntaxKind.SingleLineCommentTrivia) {
+                writer.writeLine(); // still write a newline for single-line comments, so closing tokens aren't written on the same line
+            }
+        }
+
         function emitTrailingCommentOfPosition(commentPos: number, commentEnd: number, _kind: SyntaxKind, hasTrailingNewLine: boolean) {
             // trailing comments of a position are emitted at /*trailing comment1 */space/*trailing comment*/space
 

diff --git a/src/compiler/transformers/module/module.ts b/src/compiler/transformers/module/module.ts
@@ -1790,7 +1790,7 @@ namespace ts {
                         const name = importDeclaration.propertyName || importDeclaration.name;
                         return setTextRange(
                             factory.createPropertyAccessExpression(
-                                factory.getGeneratedNameForNode(importDeclaration.parent.parent.parent),
+                                factory.getGeneratedNameForNode(importDeclaration.parent?.parent?.parent || importDeclaration),
                                 factory.cloneNode(name)
                             ),
                             /*location*/ node

diff --git a/src/compiler/transformers/module/system.ts b/src/compiler/transformers/module/system.ts
@@ -1678,7 +1678,7 @@ namespace ts {
                             factory.createPropertyAssignment(
                                 factory.cloneNode(name),
                                 factory.createPropertyAccessExpression(
-                                    factory.getGeneratedNameForNode(importDeclaration.parent.parent.parent),
+                                    factory.getGeneratedNameForNode(importDeclaration.parent?.parent?.parent || importDeclaration),
                                     factory.cloneNode(importDeclaration.propertyName || importDeclaration.name)
                                 ),
                             ),
@@ -1747,7 +1747,7 @@ namespace ts {
                     else if (isImportSpecifier(importDeclaration)) {
                         return setTextRange(
                             factory.createPropertyAccessExpression(
-                                factory.getGeneratedNameForNode(importDeclaration.parent.parent.parent),
+                                factory.getGeneratedNameForNode(importDeclaration.parent?.parent?.parent || importDeclaration),
                                 factory.cloneNode(importDeclaration.propertyName || importDeclaration.name)
                             ),
                             /*location*/ node

diff --git a/tests/baselines/reference/commentEmittingInPreserveJsx1.js b/tests/baselines/reference/commentEmittingInPreserveJsx1.js
@@ -39,19 +39,21 @@ var React = require("react");
 </div>;
 <div>
     // Not Comment
-
+    {
+    //Comment just Fine
+    }
     // Another not Comment
 </div>;
 <div>
     // Not Comment
     {
-//Comment just Fine
-"Hi"}
+    //Comment just Fine
+    "Hi"}
     // Another not Comment
 </div>;
 <div>
     /* Not Comment */
     {
-//Comment just Fine
-"Hi"}
+    //Comment just Fine
+    "Hi"}
 </div>;
diff --git a/...baselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=commonjs).js b/...baselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=commonjs).js
@@ -0,0 +1,44 @@
+//// [commentsOnJSXExpressionsArePreserved.tsx]
+// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
+namespace JSX {}
+class Component {
+    render() {
+        return <div>
+            {/* missing */}
+            {null/* preserved */}
+            {
+                // ??? 1
+            }
+            { // ??? 2
+            }
+            {// ??? 3
+            }
+            {
+                // ??? 4
+            /* ??? 5 */}
+        </div>;
+    }
+}
+
+//// [commentsOnJSXExpressionsArePreserved.jsx]
+var Component = /** @class */ (function () {
+    function Component() {
+    }
+    Component.prototype.render = function () {
+        return <div>
+            {/* missing */}
+            {null /* preserved */}
+            {
+            // ??? 1
+            }
+            {// ??? 2
+            }
+            {// ??? 3
+            }
+            {
+            // ??? 4
+            /* ??? 5 */ }
+        </div>;
+    };
+    return Component;
+}());
diff --git a/...ines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=commonjs).symbols b/...ines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=commonjs).symbols
@@ -0,0 +1,27 @@
+=== tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx ===
+// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
+namespace JSX {}
+>JSX : Symbol(JSX, Decl(commentsOnJSXExpressionsArePreserved.tsx, 0, 0))
+
+class Component {
+>Component : Symbol(Component, Decl(commentsOnJSXExpressionsArePreserved.tsx, 1, 16))
+
+    render() {
+>render : Symbol(Component.render, Decl(commentsOnJSXExpressionsArePreserved.tsx, 2, 17))
+
+        return <div>
+            {/* missing */}
+            {null/* preserved */}
+            {
+                // ??? 1
+            }
+            { // ??? 2
+            }
+            {// ??? 3
+            }
+            {
+                // ??? 4
+            /* ??? 5 */}
+        </div>;
+    }
+}
diff --git a/...elines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=commonjs).types b/...elines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=commonjs).types
@@ -0,0 +1,30 @@
+=== tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx ===
+// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
+namespace JSX {}
+class Component {
+>Component : Component
+
+    render() {
+>render : () => any
+
+        return <div>
+><div>            {/* missing */}            {null/* preserved */}            {                // ??? 1            }            { // ??? 2            }            {// ??? 3            }            {                // ??? 4            /* ??? 5 */}        </div> : error
+>div : any
+
+            {/* missing */}
+            {null/* preserved */}
+>null : null
+            {
+                // ??? 1
+            }
+            { // ??? 2
+            }
+            {// ??? 3
+            }
+            {
+                // ??? 4
+            /* ??? 5 */}
+        </div>;
+>div : any
+    }
+}
diff --git a/...s/baselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=system).js b/...s/baselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=system).js
@@ -0,0 +1,44 @@
+//// [commentsOnJSXExpressionsArePreserved.tsx]
+// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
+namespace JSX {}
+class Component {
+    render() {
+        return <div>
+            {/* missing */}
+            {null/* preserved */}
+            {
+                // ??? 1
+            }
+            { // ??? 2
+            }
+            {// ??? 3
+            }
+            {
+                // ??? 4
+            /* ??? 5 */}
+        </div>;
+    }
+}
+
+//// [commentsOnJSXExpressionsArePreserved.jsx]
+var Component = /** @class */ (function () {
+    function Component() {
+    }
+    Component.prototype.render = function () {
+        return <div>
+            {/* missing */}
+            {null /* preserved */}
+            {
+            // ??? 1
+            }
+            {// ??? 2
+            }
+            {// ??? 3
+            }
+            {
+            // ??? 4
+            /* ??? 5 */ }
+        </div>;
+    };
+    return Component;
+}());
diff --git a/...elines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=system).symbols b/...elines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=system).symbols
@@ -0,0 +1,27 @@
+=== tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx ===
+// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
+namespace JSX {}
+>JSX : Symbol(JSX, Decl(commentsOnJSXExpressionsArePreserved.tsx, 0, 0))
+
+class Component {
+>Component : Symbol(Component, Decl(commentsOnJSXExpressionsArePreserved.tsx, 1, 16))
+
+    render() {
+>render : Symbol(Component.render, Decl(commentsOnJSXExpressionsArePreserved.tsx, 2, 17))
+
+        return <div>
+            {/* missing */}
+            {null/* preserved */}
+            {
+                // ??? 1
+            }
+            { // ??? 2
+            }
+            {// ??? 3
+            }
+            {
+                // ??? 4
+            /* ??? 5 */}
+        </div>;
+    }
+}
diff --git a/...aselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=system).types b/...aselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=system).types
@@ -0,0 +1,30 @@
+=== tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx ===
+// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
+namespace JSX {}
+class Component {
+>Component : Component
+
+    render() {
+>render : () => any
+
+        return <div>
+><div>            {/* missing */}            {null/* preserved */}            {                // ??? 1            }            { // ??? 2            }            {// ??? 3            }            {                // ??? 4            /* ??? 5 */}        </div> : error
+>div : any
+
+            {/* missing */}
+            {null/* preserved */}
+>null : null
+            {
+                // ??? 1
+            }
+            { // ??? 2
+            }
+            {// ??? 3
+            }
+            {
+                // ??? 4
+            /* ??? 5 */}
+        </div>;
+>div : any
+    }
+}
diff --git a/...ines/reference/commentsOnJSXExpressionsArePreserved(jsx=react,module=commonjs).errors.txt b/...ines/reference/commentsOnJSXExpressionsArePreserved(jsx=react,module=commonjs).errors.txt
@@ -0,0 +1,26 @@
+tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx(5,17): error TS2304: Cannot find name 'React'.
+
+
+==== tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx (1 errors) ====
+    // file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
+    namespace JSX {}
+    class Component {
+        render() {
+            return <div>
+                    ~~~
+!!! error TS2304: Cannot find name 'React'.
+                {/* missing */}
+                {null/* preserved */}
+                {
+                    // ??? 1
+                }
+                { // ??? 2
+                }
+                {// ??? 3
+                }
+                {
+                    // ??? 4
+                /* ??? 5 */}
+            </div>;
+        }
+    }