Merge pull request #24311 from amcasey/GH23287

Preserve jsx imports even when the compiler option is not set

Author: amcasey

src/harness/unittests/organizeImports.ts

@@ -506,7 +506,29 @@ D();
                 },
                 libFile);
 
-            testOrganizeImports("JsxFactoryUsed",
+            testOrganizeImports("JsxFactoryUsedJsx",
+                {
+                    path: "/test.jsx",
+                    content: `
+import { React, Other } from "react";
+
+<div/>;
+`,
+                },
+                reactLibFile);
+
+            testOrganizeImports("JsxFactoryUsedJs",
+                {
+                    path: "/test.js",
+                    content: `
+import { React, Other } from "react";
+
+<div/>;
+`,
+                },
+                reactLibFile);
+
+            testOrganizeImports("JsxFactoryUsedTsx",
                 {
                     path: "/test.tsx",
                     content: `
@@ -517,7 +539,39 @@ import { React, Other } from "react";
                 },
                 reactLibFile);
 
-            // This is descriptive, rather than normative
+            // TS files are not JSX contexts, so the parser does not treat
+            // `<div/>` as a JSX element.
+            testOrganizeImports("JsxFactoryUsedTs",
+                {
+                    path: "/test.ts",
+                    content: `
+import { React, Other } from "react";
+
+<div/>;
+`,
+                },
+                reactLibFile);
+
+            testOrganizeImports("JsxFactoryUnusedJsx",
+                {
+                    path: "/test.jsx",
+                    content: `
+import { React, Other } from "react";
+`,
+                },
+                reactLibFile);
+
+            // Note: Since the file extension does not end with "x", the jsx compiler option
+            // will not be enabled.  The import should be retained regardless.
+            testOrganizeImports("JsxFactoryUnusedJs",
+                {
+                    path: "/test.js",
+                    content: `
+import { React, Other } from "react";
+`,
+                },
+                reactLibFile);
+
             testOrganizeImports("JsxFactoryUnusedTsx",
                 {
                     path: "/test.tsx",

src/services/organizeImports.ts

@@ -92,7 +92,7 @@ namespace ts.OrganizeImports {
     function removeUnusedImports(oldImports: ReadonlyArray<ImportDeclaration>, sourceFile: SourceFile, program: Program) {
         const typeChecker = program.getTypeChecker();
         const jsxNamespace = typeChecker.getJsxNamespace();
-        const jsxContext = sourceFile.languageVariant === LanguageVariant.JSX && program.getCompilerOptions().jsx;
+        const jsxElementsPresent = !!(sourceFile.transformFlags & TransformFlags.ContainsJsx);
 
         const usedImports: ImportDeclaration[] = [];
 
@@ -138,8 +138,8 @@ namespace ts.OrganizeImports {
         return usedImports;
 
         function isDeclarationUsed(identifier: Identifier) {
-            // The JSX factory symbol is always used.
-            return jsxContext && (identifier.text === jsxNamespace) || FindAllReferences.Core.isSymbolReferencedInFile(identifier, typeChecker, sourceFile);
+            // The JSX factory symbol is always used if JSX elements are present - even if they are not allowed.
+            return jsxElementsPresent && (identifier.text === jsxNamespace) || FindAllReferences.Core.isSymbolReferencedInFile(identifier, typeChecker, sourceFile);
         }
     }
 

tests/baselines/reference/organizeImports/JsxFactoryUnusedJs.ts

@@ -0,0 +1,6 @@
+// ==ORIGINAL==
+
+import { React, Other } from "react";
+
+// ==ORGANIZED==
+

tests/baselines/reference/organizeImports/JsxFactoryUnusedJsx.ts

@@ -0,0 +1,6 @@
+// ==ORIGINAL==
+
+import { React, Other } from "react";
+
+// ==ORGANIZED==
+

tests/baselines/reference/organizeImports/JsxFactoryUnusedTsx.ts

@@ -4,4 +4,3 @@ import { React, Other } from "react";
 
 // ==ORGANIZED==
 
-import { React } from "react";

tests/baselines/reference/organizeImports/JsxFactoryUsed.ts renamed to tests/baselines/reference/organizeImports/JsxFactoryUsedJs.ts

@@ -0,0 +1,11 @@
+// ==ORIGINAL==
+
+import { React, Other } from "react";
+
+<div/>;
+
+// ==ORGANIZED==
+
+import { React } from "react";
+
+<div/>;

tests/baselines/reference/organizeImports/JsxFactoryUsedJsx.ts

@@ -0,0 +1,10 @@
+// ==ORIGINAL==
+
+import { React, Other } from "react";
+
+<div/>;
+
+// ==ORGANIZED==
+
+
+<div/>;

tests/baselines/reference/organizeImports/JsxFactoryUsedTs.ts

@@ -0,0 +1,11 @@
+// ==ORIGINAL==
+
+import { React, Other } from "react";
+
+<div/>;
+
+// ==ORGANIZED==
+
+import { React } from "react";
+
+<div/>;