diff --git a/.changeset/chilled-oranges-sit.md b/.changeset/chilled-oranges-sit.md index 48ac726b..a1977526 100644 --- a/.changeset/chilled-oranges-sit.md +++ b/.changeset/chilled-oranges-sit.md @@ -15,6 +15,28 @@ Now we ensure new imports of types use the `type` modifier: +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. diff --git a/transforms/__tests__/deprecated-react-fragment.js b/transforms/__tests__/deprecated-react-fragment.js index 490b1368..9f0c0489 100644 --- a/transforms/__tests__/deprecated-react-fragment.js +++ b/transforms/__tests__/deprecated-react-fragment.js @@ -47,6 +47,22 @@ describe("transform deprecated-react-node-array", () => { `); }); + 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; + }" + `); + }); + test("named import with existing ReactNode import", () => { expect( applyTransform(` @@ -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 9bcf2cdd..21fa8fd9 100644 --- a/transforms/__tests__/deprecated-react-node-array.js +++ b/transforms/__tests__/deprecated-react-node-array.js @@ -47,6 +47,22 @@ describe("transform deprecated-react-node-array", () => { `); }); + 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; + }" + `); + }); + test("named import with existing ReactNode import", () => { expect( applyTransform(` @@ -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__/scoped-jsx.js b/transforms/__tests__/scoped-jsx.js index 709e812c..6ebbb62c 100644 --- a/transforms/__tests__/scoped-jsx.js +++ b/transforms/__tests__/scoped-jsx.js @@ -38,6 +38,31 @@ describe("transform scoped-jsx", () => { `); }); + 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;" + `); + }); + test("existing namespace import", () => { expect( applyTransform(` @@ -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,7 +84,8 @@ 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;" `); }); @@ -76,6 +103,20 @@ describe("transform scoped-jsx", () => { `); }); + 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;" + `); + }); + test("existing named import", () => { expect( applyTransform(` @@ -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 7c2b533a..166e22e2 100644 --- a/transforms/deprecated-react-fragment.js +++ b/transforms/deprecated-react-fragment.js @@ -1,5 +1,8 @@ const parseSync = require("./utils/parseSync"); - +const { + addReactTypeImport, + findReactImportForNewImports, +} = require("./utils/newReactImports"); /** * @type {import('jscodeshift').Transform} */ @@ -29,10 +32,13 @@ const deprecatedReactFragmentTransform = (file, api) => { if (hasReactNodeImport.length > 0) { reactFragmentImports.remove(); } else { - reactFragmentImports.replaceWith(() => { + reactFragmentImports.replaceWith((path) => { const importSpecifier = j.importSpecifier(j.identifier("ReactNode")); - // @ts-expect-error -- Missing types in jscodeshift. Babel uses `importKind`: https://astexplorer.net/#/gist/a76bd35f28483a467fef29d3c63aac9b/0e7ba6688fc09bd11b92197349b2384bb4c94574 - importSpecifier.importKind = "type"; + 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 20a8945f..ed6cbbc6 100644 --- a/transforms/deprecated-react-node-array.js +++ b/transforms/deprecated-react-node-array.js @@ -29,10 +29,14 @@ const deprecatedReactNodeArrayTransform = (file, api) => { if (hasReactNodeImport.length > 0) { reactNodeArrayImports.remove(); } else { - reactNodeArrayImports.replaceWith(() => { + reactNodeArrayImports.replaceWith((path) => { const importSpecifier = j.importSpecifier(j.identifier("ReactNode")); - // @ts-expect-error -- Missing types in jscodeshift. Babel uses `importKind`: https://astexplorer.net/#/gist/a76bd35f28483a467fef29d3c63aac9b/0e7ba6688fc09bd11b92197349b2384bb4c94574 - importSpecifier.importKind = "type"; + + 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 d49c4664..69036cd0 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" }, }); @@ -66,14 +34,13 @@ const deprecatedReactChildTransform = (file, api) => { if (jsxImportSpecifier.length === 0) { hasChanges = true; - const importSpecifier = j.importSpecifier(j.identifier("JSX")); - // @ts-expect-error -- Missing types in jscodeshift. Babel uses `importKind`: https://astexplorer.net/#/gist/a76bd35f28483a467fef29d3c63aac9b/0e7ba6688fc09bd11b92197349b2384bb4c94574 - importSpecifier.importKind = "type"; - const hasExistingReactImport = reactImport.length > 0; if (hasExistingReactImport) { - reactImport.get("specifiers").value.push(importSpecifier); + addReactTypeImport(j, reactImport); } else { + const importSpecifier = j.importSpecifier(j.identifier("JSX")); + // @ts-expect-error -- Missing types in jscodeshift. Babel uses `importKind`: https://astexplorer.net/#/gist/a76bd35f28483a467fef29d3c63aac9b/0e7ba6688fc09bd11b92197349b2384bb4c94574 + importSpecifier.importKind = "type"; const jsxNamespaceImport = j.importDeclaration( [importSpecifier], j.stringLiteral("react"), 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 };