From edd31a15056d85f2ee175770e00e971ac806f855 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Mon, 21 May 2018 16:57:18 -0700 Subject: [PATCH 1/2] Preserver jsx imports even when the compiler option is not set ...based on feedback from VS Code users. Fixes #23287 --- src/harness/unittests/organizeImports.ts | 20 +++++++++++++++++++ src/services/organizeImports.ts | 2 +- .../organizeImports/JsxFactoryUnusedJs.ts | 7 +++++++ .../organizeImports/JsxFactoryUnusedJsx.ts | 7 +++++++ 4 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/organizeImports/JsxFactoryUnusedJs.ts create mode 100644 tests/baselines/reference/organizeImports/JsxFactoryUnusedJsx.ts diff --git a/src/harness/unittests/organizeImports.ts b/src/harness/unittests/organizeImports.ts index 94f99856777e7..61b4e240d9b68 100644 --- a/src/harness/unittests/organizeImports.ts +++ b/src/harness/unittests/organizeImports.ts @@ -513,6 +513,26 @@ D(); import { React, Other } from "react";
; +`, + }, + 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); diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 99bac64973f80..30f7f1584a419 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -92,7 +92,7 @@ namespace ts.OrganizeImports { function removeUnusedImports(oldImports: ReadonlyArray, sourceFile: SourceFile, program: Program) { const typeChecker = program.getTypeChecker(); const jsxNamespace = typeChecker.getJsxNamespace(); - const jsxContext = sourceFile.languageVariant === LanguageVariant.JSX && program.getCompilerOptions().jsx; + const jsxContext = sourceFile.languageVariant === LanguageVariant.JSX; const usedImports: ImportDeclaration[] = []; diff --git a/tests/baselines/reference/organizeImports/JsxFactoryUnusedJs.ts b/tests/baselines/reference/organizeImports/JsxFactoryUnusedJs.ts new file mode 100644 index 0000000000000..6a97e7f660ae1 --- /dev/null +++ b/tests/baselines/reference/organizeImports/JsxFactoryUnusedJs.ts @@ -0,0 +1,7 @@ +// ==ORIGINAL== + +import { React, Other } from "react"; + +// ==ORGANIZED== + +import { React } from "react"; diff --git a/tests/baselines/reference/organizeImports/JsxFactoryUnusedJsx.ts b/tests/baselines/reference/organizeImports/JsxFactoryUnusedJsx.ts new file mode 100644 index 0000000000000..6a97e7f660ae1 --- /dev/null +++ b/tests/baselines/reference/organizeImports/JsxFactoryUnusedJsx.ts @@ -0,0 +1,7 @@ +// ==ORIGINAL== + +import { React, Other } from "react"; + +// ==ORGANIZED== + +import { React } from "react"; From 2e0cc63067cdb72d290c0b3a8de99027d152e5d5 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Tue, 22 May 2018 10:51:19 -0700 Subject: [PATCH 2/2] Check TransformFlags.ContainsJsx, rather than LanguageVariant.JSX --- src/harness/unittests/organizeImports.ts | 38 ++++++++++++++++++- src/services/organizeImports.ts | 6 +-- .../organizeImports/JsxFactoryUnusedJs.ts | 1 - .../organizeImports/JsxFactoryUnusedJsx.ts | 1 - .../organizeImports/JsxFactoryUnusedTsx.ts | 1 - ...{JsxFactoryUsed.ts => JsxFactoryUsedJs.ts} | 0 .../organizeImports/JsxFactoryUsedJsx.ts | 11 ++++++ .../organizeImports/JsxFactoryUsedTs.ts | 10 +++++ .../organizeImports/JsxFactoryUsedTsx.ts | 11 ++++++ 9 files changed, 71 insertions(+), 8 deletions(-) rename tests/baselines/reference/organizeImports/{JsxFactoryUsed.ts => JsxFactoryUsedJs.ts} (100%) create mode 100644 tests/baselines/reference/organizeImports/JsxFactoryUsedJsx.ts create mode 100644 tests/baselines/reference/organizeImports/JsxFactoryUsedTs.ts create mode 100644 tests/baselines/reference/organizeImports/JsxFactoryUsedTsx.ts diff --git a/src/harness/unittests/organizeImports.ts b/src/harness/unittests/organizeImports.ts index 61b4e240d9b68..9dea2d0d51e85 100644 --- a/src/harness/unittests/organizeImports.ts +++ b/src/harness/unittests/organizeImports.ts @@ -506,12 +506,47 @@ D(); }, libFile); - testOrganizeImports("JsxFactoryUsed", + testOrganizeImports("JsxFactoryUsedJsx", + { + path: "/test.jsx", + content: ` +import { React, Other } from "react"; + +
; +`, + }, + reactLibFile); + + testOrganizeImports("JsxFactoryUsedJs", + { + path: "/test.js", + content: ` +import { React, Other } from "react"; + +
; +`, + }, + reactLibFile); + + testOrganizeImports("JsxFactoryUsedTsx", { path: "/test.tsx", content: ` import { React, Other } from "react"; +
; +`, + }, + reactLibFile); + + // TS files are not JSX contexts, so the parser does not treat + // `
` as a JSX element. + testOrganizeImports("JsxFactoryUsedTs", + { + path: "/test.ts", + content: ` +import { React, Other } from "react"; +
; `, }, @@ -537,7 +572,6 @@ import { React, Other } from "react"; }, reactLibFile); - // This is descriptive, rather than normative testOrganizeImports("JsxFactoryUnusedTsx", { path: "/test.tsx", diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 30f7f1584a419..225e9fae57d76 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -92,7 +92,7 @@ namespace ts.OrganizeImports { function removeUnusedImports(oldImports: ReadonlyArray, sourceFile: SourceFile, program: Program) { const typeChecker = program.getTypeChecker(); const jsxNamespace = typeChecker.getJsxNamespace(); - const jsxContext = sourceFile.languageVariant === LanguageVariant.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); } } diff --git a/tests/baselines/reference/organizeImports/JsxFactoryUnusedJs.ts b/tests/baselines/reference/organizeImports/JsxFactoryUnusedJs.ts index 6a97e7f660ae1..60afb95a19257 100644 --- a/tests/baselines/reference/organizeImports/JsxFactoryUnusedJs.ts +++ b/tests/baselines/reference/organizeImports/JsxFactoryUnusedJs.ts @@ -4,4 +4,3 @@ import { React, Other } from "react"; // ==ORGANIZED== -import { React } from "react"; diff --git a/tests/baselines/reference/organizeImports/JsxFactoryUnusedJsx.ts b/tests/baselines/reference/organizeImports/JsxFactoryUnusedJsx.ts index 6a97e7f660ae1..60afb95a19257 100644 --- a/tests/baselines/reference/organizeImports/JsxFactoryUnusedJsx.ts +++ b/tests/baselines/reference/organizeImports/JsxFactoryUnusedJsx.ts @@ -4,4 +4,3 @@ import { React, Other } from "react"; // ==ORGANIZED== -import { React } from "react"; diff --git a/tests/baselines/reference/organizeImports/JsxFactoryUnusedTsx.ts b/tests/baselines/reference/organizeImports/JsxFactoryUnusedTsx.ts index 6a97e7f660ae1..60afb95a19257 100644 --- a/tests/baselines/reference/organizeImports/JsxFactoryUnusedTsx.ts +++ b/tests/baselines/reference/organizeImports/JsxFactoryUnusedTsx.ts @@ -4,4 +4,3 @@ import { React, Other } from "react"; // ==ORGANIZED== -import { React } from "react"; diff --git a/tests/baselines/reference/organizeImports/JsxFactoryUsed.ts b/tests/baselines/reference/organizeImports/JsxFactoryUsedJs.ts similarity index 100% rename from tests/baselines/reference/organizeImports/JsxFactoryUsed.ts rename to tests/baselines/reference/organizeImports/JsxFactoryUsedJs.ts diff --git a/tests/baselines/reference/organizeImports/JsxFactoryUsedJsx.ts b/tests/baselines/reference/organizeImports/JsxFactoryUsedJsx.ts new file mode 100644 index 0000000000000..430a5b12cd4d4 --- /dev/null +++ b/tests/baselines/reference/organizeImports/JsxFactoryUsedJsx.ts @@ -0,0 +1,11 @@ +// ==ORIGINAL== + +import { React, Other } from "react"; + +
; + +// ==ORGANIZED== + +import { React } from "react"; + +
; diff --git a/tests/baselines/reference/organizeImports/JsxFactoryUsedTs.ts b/tests/baselines/reference/organizeImports/JsxFactoryUsedTs.ts new file mode 100644 index 0000000000000..74da2f99139e0 --- /dev/null +++ b/tests/baselines/reference/organizeImports/JsxFactoryUsedTs.ts @@ -0,0 +1,10 @@ +// ==ORIGINAL== + +import { React, Other } from "react"; + +
; + +// ==ORGANIZED== + + +
; diff --git a/tests/baselines/reference/organizeImports/JsxFactoryUsedTsx.ts b/tests/baselines/reference/organizeImports/JsxFactoryUsedTsx.ts new file mode 100644 index 0000000000000..430a5b12cd4d4 --- /dev/null +++ b/tests/baselines/reference/organizeImports/JsxFactoryUsedTsx.ts @@ -0,0 +1,11 @@ +// ==ORIGINAL== + +import { React, Other } from "react"; + +
; + +// ==ORGANIZED== + +import { React } from "react"; + +
;