Ensure we don't produce syntax errors

.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.

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<ReactNode>;
+			}"
+	`);
+	});
+
 	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<ReactNode>;
+			}"
+	`);
+	});
+
 	test("false-negative named renamed import", () => {
 		expect(
 			applyTransform(`

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<ReactNode>;
+			}"
+	`);
+	});
+
 	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<ReactNode>;
+			}"
+	`);
+	});
+
 	test("false-negative named renamed import", () => {
 		expect(
 			applyTransform(`

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<A, B>;"
+		import { type JSX } from "react";
+
+		declare const attributes: JSX.LibraryManagedAttributes<A, B>;"
 	`);
 	});
 });

transforms/deprecated-react-fragment.js

@@ -1,5 +1,4 @@
 const parseSync = require("./utils/parseSync");
-
 /**
  * @type {import('jscodeshift').Transform}
  */
@@ -29,10 +28,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;
 		});

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;
 		});

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,45 +25,22 @@ 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" },
 		});
 
 		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"),

transforms/utils/newReactImports.js

@@ -0,0 +1,47 @@
+/**
+ * @param {import('jscodeshift').API['jscodeshift']} j
+ * @param {import('jscodeshift').Collection} ast
+ * @returns {import('jscodeshift').Collection<import('jscodeshift').ImportDeclaration>}
+ */
+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<import('jscodeshift').ImportDeclaration>} 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 };