diff --git a/.changeset/chilled-oranges-sit.md b/.changeset/chilled-oranges-sit.md new file mode 100644 index 00000000..a1977526 --- /dev/null +++ b/.changeset/chilled-oranges-sit.md @@ -0,0 +1,50 @@ +--- +"types-react-codemod": patch +--- + +Ensure added imports of types use the `type` modifier + +If we'd previously add an import to `ReactNode` (e.g. in `deprecated-react-fragment`), +the codemod would import it as a value. +This breaks TypeScript projects using `verbatimModuleSyntax` as well as projects enforcing `type` imports for types. + +Now we ensure new imports of types use the `type` modifier: + +```diff +-import { ReactNode } from 'react' ++import { type ReactNode } from 'react' +``` + +This also changes how we transform the deprecated global JSX namespace. +Instead of rewriting each usage, we opt for adding another import. +The guiding principle being that we keep the changes minimal in a codemod. + +Before: + +```diff +import * as React from 'react' + +-const element: JSX.Element ++const element: React.JSX.Element +``` + +After: + +```diff +import * as React from 'react' ++import { type JSX } from 'react' + +const element: JSX.Element +``` + +Note that rewriting of imports does not change the modifier. +For example, the `deprecated-vfc-codemod` rewrites `VFC` identifiers to `FC`. +If the import of `VFC` had no `type` modifier, the codemod will not add one. + +This affects the following codemods: + +- `deprecated-react-fragment` +- `deprecated-react-node-array` +- `scoped-jsx` + +`type` modifiers for import specifiers require [TypeScript 4.5 which has reached EOL](https://github.com/DefinitelyTyped/DefinitelyTyped#support-window in DefinitelyTyped) which is a strong signal that you should upgrade to at least TypeScript 4.6 by now. diff --git a/transforms/__tests__/deprecated-react-fragment.js b/transforms/__tests__/deprecated-react-fragment.js index f8338bab..9f0c0489 100644 --- a/transforms/__tests__/deprecated-react-fragment.js +++ b/transforms/__tests__/deprecated-react-fragment.js @@ -40,7 +40,23 @@ describe("transform deprecated-react-node-array", () => { } `), ).toMatchInlineSnapshot(` - "import { ReactNode } from 'react'; + "import { type ReactNode } from 'react'; + interface Props { + children?: Iterable; + }" + `); + }); + + test("named type import", () => { + expect( + applyTransform(` + import type { ReactFragment } from 'react'; + interface Props { + children?: ReactFragment; + } + `), + ).toMatchInlineSnapshot(` + "import type { ReactNode } from 'react'; interface Props { children?: Iterable; }" @@ -63,6 +79,22 @@ describe("transform deprecated-react-node-array", () => { `); }); + test("named import with existing ReactNode type import", () => { + expect( + applyTransform(` + import { ReactFragment, type ReactNode } from 'react'; + interface Props { + children?: ReactFragment; + } + `), + ).toMatchInlineSnapshot(` + "import { type ReactNode } from 'react'; + interface Props { + children?: Iterable; + }" + `); + }); + test("false-negative named renamed import", () => { expect( applyTransform(` diff --git a/transforms/__tests__/deprecated-react-node-array.js b/transforms/__tests__/deprecated-react-node-array.js index fea0e982..21fa8fd9 100644 --- a/transforms/__tests__/deprecated-react-node-array.js +++ b/transforms/__tests__/deprecated-react-node-array.js @@ -40,7 +40,23 @@ describe("transform deprecated-react-node-array", () => { } `), ).toMatchInlineSnapshot(` - "import { ReactNode } from 'react'; + "import { type ReactNode } from 'react'; + interface Props { + children?: ReadonlyArray; + }" + `); + }); + + test("named type import", () => { + expect( + applyTransform(` + import type { ReactNodeArray } from 'react'; + interface Props { + children?: ReactNodeArray; + } + `), + ).toMatchInlineSnapshot(` + "import type { ReactNode } from 'react'; interface Props { children?: ReadonlyArray; }" @@ -63,6 +79,22 @@ describe("transform deprecated-react-node-array", () => { `); }); + test("named import with existing ReactNode type import", () => { + expect( + applyTransform(` + import { ReactNodeArray, type ReactNode } from 'react'; + interface Props { + children?: ReactNodeArray; + } + `), + ).toMatchInlineSnapshot(` + "import { type ReactNode } from 'react'; + interface Props { + children?: ReadonlyArray; + }" + `); + }); + test("false-negative named renamed import", () => { expect( applyTransform(` diff --git a/transforms/__tests__/deprecated-sfc-element.js b/transforms/__tests__/deprecated-sfc-element.js index 3b5ae30c..c00a58f0 100644 --- a/transforms/__tests__/deprecated-sfc-element.js +++ b/transforms/__tests__/deprecated-sfc-element.js @@ -39,6 +39,18 @@ describe("transform deprecated-sfc-element", () => { `); }); + test("named typew import", () => { + expect( + applyTransform(` + import { type SFCElement } from 'react'; + SFCElement; + `), + ).toMatchInlineSnapshot(` + "import { type FunctionComponentElement } from 'react'; + FunctionComponentElement;" + `); + }); + test("named renamed import", () => { expect( applyTransform(` diff --git a/transforms/__tests__/deprecated-sfc.js b/transforms/__tests__/deprecated-sfc.js index cfb7a949..3462df4c 100644 --- a/transforms/__tests__/deprecated-sfc.js +++ b/transforms/__tests__/deprecated-sfc.js @@ -35,6 +35,18 @@ describe("transform deprecated-sfc", () => { `); }); + test("named type import", () => { + expect( + applyTransform(` + import { type SFC } from 'react'; + SFC; + `), + ).toMatchInlineSnapshot(` + "import { type FC } from 'react'; + FC;" + `); + }); + test("named renamed import", () => { expect( applyTransform(` diff --git a/transforms/__tests__/deprecated-stateless-component.js b/transforms/__tests__/deprecated-stateless-component.js index 151e7ea5..5262b50c 100644 --- a/transforms/__tests__/deprecated-stateless-component.js +++ b/transforms/__tests__/deprecated-stateless-component.js @@ -39,6 +39,18 @@ describe("transform deprecated-stateless-component", () => { `); }); + test("named type import", () => { + expect( + applyTransform(` + import { type StatelessComponent } from 'react'; + StatelessComponent; + `), + ).toMatchInlineSnapshot(` + "import { type FunctionComponent } from 'react'; + FunctionComponent;" + `); + }); + test("named renamed import", () => { expect( applyTransform(` diff --git a/transforms/__tests__/deprecated-void-function-component.js b/transforms/__tests__/deprecated-void-function-component.js index 24d2ebc8..380fa282 100644 --- a/transforms/__tests__/deprecated-void-function-component.js +++ b/transforms/__tests__/deprecated-void-function-component.js @@ -41,6 +41,20 @@ describe("transform deprecated-void-function-component", () => { `); }); + test("named type import", () => { + expect( + applyTransform(` + import { type VFC, type VoidFunctionComponent } from 'react'; + VFC; + VoidFunctionComponent; + `), + ).toMatchInlineSnapshot(` + "import { type FC, type FunctionComponent } from 'react'; + FC; + FunctionComponent;" + `); + }); + test("named renamed import", () => { expect( applyTransform(` diff --git a/transforms/__tests__/scoped-jsx.js b/transforms/__tests__/scoped-jsx.js index af47c2a9..c0d1837f 100644 --- a/transforms/__tests__/scoped-jsx.js +++ b/transforms/__tests__/scoped-jsx.js @@ -33,7 +33,32 @@ describe("transform scoped-jsx", () => { declare const element: JSX.Element; `), ).toMatchInlineSnapshot(` - "import { JSX } from "react"; + "import type { JSX } from "react"; + declare const element: JSX.Element;" + `); + }); + + test("existing default import", () => { + expect( + applyTransform(` + import React from 'react'; + declare const element: JSX.Element; + `), + ).toMatchInlineSnapshot(` + "import React, { type JSX } from 'react'; + declare const element: JSX.Element;" + `); + }); + + test("existing type default import", () => { + expect( + applyTransform(` + import type React from 'react'; + declare const element: JSX.Element; + `), + ).toMatchInlineSnapshot(` + "import type React from 'react'; + import type { JSX } from "react"; declare const element: JSX.Element;" `); }); @@ -46,7 +71,8 @@ describe("transform scoped-jsx", () => { `), ).toMatchInlineSnapshot(` "import * as React from 'react'; - declare const element: React.JSX.Element;" + import type { JSX } from "react"; + declare const element: JSX.Element;" `); }); @@ -58,11 +84,12 @@ describe("transform scoped-jsx", () => { `), ).toMatchInlineSnapshot(` "import type * as React from 'react'; - declare const element: React.JSX.Element;" + import type { JSX } from "react"; + declare const element: JSX.Element;" `); }); - test("existing named import", () => { + test("existing named import with other named imports", () => { expect( applyTransform(` import { ReactNode } from 'react'; @@ -70,7 +97,21 @@ describe("transform scoped-jsx", () => { declare const node: ReactNode; `), ).toMatchInlineSnapshot(` - "import { ReactNode, JSX } from 'react'; + "import { ReactNode, type JSX } from 'react'; + declare const element: JSX.Element; + declare const node: ReactNode;" + `); + }); + + test("existing named import with other named type imports", () => { + expect( + applyTransform(` + import type { ReactNode } from 'react'; + declare const element: JSX.Element; + declare const node: ReactNode; + `), + ).toMatchInlineSnapshot(` + "import type { ReactNode, JSX } from 'react'; declare const element: JSX.Element; declare const node: ReactNode;" `); @@ -95,7 +136,7 @@ describe("transform scoped-jsx", () => { declare const element: JSX.Element; `), ).toMatchInlineSnapshot(` - "import { JSX } from "react"; + "import type { JSX } from "react"; const React = require('react'); declare const element: JSX.Element;" `); @@ -113,7 +154,7 @@ describe("transform scoped-jsx", () => { "import {} from 'react-dom' import {} from '@testing-library/react' - import { JSX } from "react"; + import type { JSX } from "react"; declare const element: JSX.Element;" `); @@ -129,7 +170,9 @@ describe("transform scoped-jsx", () => { ).toMatchInlineSnapshot(` "import * as React from 'react' - declare const attributes: React.JSX.LibraryManagedAttributes;" + import type { JSX } from "react"; + + declare const attributes: JSX.LibraryManagedAttributes;" `); }); }); diff --git a/transforms/deprecated-react-fragment.js b/transforms/deprecated-react-fragment.js index 641cbc28..9da7a28f 100644 --- a/transforms/deprecated-react-fragment.js +++ b/transforms/deprecated-react-fragment.js @@ -1,5 +1,4 @@ const parseSync = require("./utils/parseSync"); - /** * @type {import('jscodeshift').Transform} */ @@ -29,8 +28,15 @@ const deprecatedReactFragmentTransform = (file, api) => { if (hasReactNodeImport.length > 0) { reactFragmentImports.remove(); } else { - reactFragmentImports.replaceWith(() => { - return j.importSpecifier(j.identifier("ReactNode")); + reactFragmentImports.replaceWith((path) => { + const importSpecifier = j.importSpecifier(j.identifier("ReactNode")); + const importDeclaration = path.parentPath.parentPath.value; + if (importDeclaration.importKind !== "type") { + // @ts-expect-error -- Missing types in jscodeshift. Babel uses `importKind`: https://astexplorer.net/#/gist/a76bd35f28483a467fef29d3c63aac9b/0e7ba6688fc09bd11b92197349b2384bb4c94574 + importSpecifier.importKind = "type"; + } + + return importSpecifier; }); } diff --git a/transforms/deprecated-react-node-array.js b/transforms/deprecated-react-node-array.js index 963cce23..ed6cbbc6 100644 --- a/transforms/deprecated-react-node-array.js +++ b/transforms/deprecated-react-node-array.js @@ -29,8 +29,16 @@ const deprecatedReactNodeArrayTransform = (file, api) => { if (hasReactNodeImport.length > 0) { reactNodeArrayImports.remove(); } else { - reactNodeArrayImports.replaceWith(() => { - return j.importSpecifier(j.identifier("ReactNode")); + reactNodeArrayImports.replaceWith((path) => { + const importSpecifier = j.importSpecifier(j.identifier("ReactNode")); + + const importDeclaration = path.parentPath.parentPath.value; + if (importDeclaration.importKind !== "type") { + // @ts-expect-error -- Missing types in jscodeshift. Babel uses `importKind`: https://astexplorer.net/#/gist/a76bd35f28483a467fef29d3c63aac9b/0e7ba6688fc09bd11b92197349b2384bb4c94574 + importSpecifier.importKind = "type"; + } + + return importSpecifier; }); } diff --git a/transforms/scoped-jsx.js b/transforms/scoped-jsx.js index 816fffbd..f61f917b 100644 --- a/transforms/scoped-jsx.js +++ b/transforms/scoped-jsx.js @@ -1,4 +1,8 @@ const parseSync = require("./utils/parseSync"); +const { + addReactTypeImport, + findReactImportForNewImports, +} = require("./utils/newReactImports"); /** * @type {import('jscodeshift').Transform} @@ -7,20 +11,6 @@ const deprecatedReactChildTransform = (file, api) => { const j = api.jscodeshift; const ast = parseSync(file); - /** - * @type {string | null} - */ - let reactNamespaceName = null; - ast.find(j.ImportDeclaration).forEach((importDeclaration) => { - const node = importDeclaration.value; - if ( - node.source.value === "react" && - node.specifiers?.[0]?.type === "ImportNamespaceSpecifier" - ) { - reactNamespaceName = node.specifiers[0].local?.name ?? null; - } - }); - const globalNamespaceReferences = ast.find(j.TSTypeReference, (node) => { const { typeName } = node; @@ -35,30 +25,8 @@ const deprecatedReactChildTransform = (file, api) => { }); let hasChanges = false; - if (reactNamespaceName !== null && globalNamespaceReferences.length > 0) { - hasChanges = true; - - globalNamespaceReferences.replaceWith((typeReference) => { - const namespaceMember = typeReference - .get("typeName") - .get("right") - .get("name").value; - - return j.tsTypeReference( - j.tsQualifiedName( - j.tsQualifiedName( - j.identifier(/** @type {string} */ (reactNamespaceName)), - j.identifier("JSX"), - ), - j.identifier(namespaceMember), - ), - typeReference.value.typeParameters, - ); - }); - } else if (globalNamespaceReferences.length > 0) { - const reactImport = ast.find(j.ImportDeclaration, { - source: { value: "react" }, - }); + if (globalNamespaceReferences.length > 0) { + const reactImport = findReactImportForNewImports(j, ast); const jsxImportSpecifier = reactImport.find(j.ImportSpecifier, { imported: { name: "JSX" }, }); @@ -68,13 +36,14 @@ const deprecatedReactChildTransform = (file, api) => { const hasExistingReactImport = reactImport.length > 0; if (hasExistingReactImport) { - reactImport - .get("specifiers") - .value.push(j.importSpecifier(j.identifier("JSX"))); + addReactTypeImport(j, reactImport); } else { + const importSpecifier = j.importSpecifier(j.identifier("JSX")); const jsxNamespaceImport = j.importDeclaration( - [j.importSpecifier(j.identifier("JSX"))], + [importSpecifier], j.stringLiteral("react"), + // Not using inline type import because it violates https://typescript-eslint.io/rules/no-import-type-side-effects/ + "type", ); const lastImport = ast.find(j.ImportDeclaration).at(-1); diff --git a/transforms/utils/newReactImports.js b/transforms/utils/newReactImports.js new file mode 100644 index 00000000..4d86fa33 --- /dev/null +++ b/transforms/utils/newReactImports.js @@ -0,0 +1,47 @@ +/** + * @param {import('jscodeshift').API['jscodeshift']} j + * @param {import('jscodeshift').Collection} ast + * @returns {import('jscodeshift').Collection} + */ +function findReactImportForNewImports(j, ast) { + return ast + .find(j.ImportDeclaration, { + source: { value: "react" }, + }) + .filter((path) => { + const importDeclaration = path.node; + // Filter out + // - `import * as React from 'react';` + // - `import type * as React from 'react';` + // - `import type React from 'react';` + // All other patterns can take a named import + if (importDeclaration.specifiers?.length === 1) { + const specifier = importDeclaration.specifiers[0]; + const isDefaultTypeImport = + specifier.type === "ImportDefaultSpecifier" && + importDeclaration.importKind === "type"; + + return ( + !isDefaultTypeImport && specifier.type !== "ImportNamespaceSpecifier" + ); + } + return true; + }); +} + +/** + * @param {import('jscodeshift').API['jscodeshift']} j + * @param {import('jscodeshift').Collection} reactImport + */ +function addReactTypeImport(j, reactImport) { + const importSpecifier = j.importSpecifier(j.identifier("JSX")); + + if (reactImport.get("importKind").value !== "type") { + // @ts-expect-error -- Missing types in jscodeshift. Babel uses `importKind`: https://astexplorer.net/#/gist/a76bd35f28483a467fef29d3c63aac9b/0e7ba6688fc09bd11b92197349b2384bb4c94574 + importSpecifier.importKind = "type"; + } + + reactImport.get("specifiers").value.push(importSpecifier); +} + +module.exports = { addReactTypeImport, findReactImportForNewImports };