fix: Ensure added imports of types use the type modifier (#343)

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

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<ReactNode>;
+			}"
+	`);
+	});
+
+	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>;
 			}"
@@ -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

@@ -40,7 +40,23 @@ describe("transform deprecated-react-node-array", () => {
 			}
     `),
 		).toMatchInlineSnapshot(`
-		"import { ReactNode } from 'react';
+		"import { type ReactNode } from 'react';
+		interface Props {
+				children?: ReadonlyArray<ReactNode>;
+			}"
+	`);
+	});
+
+	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>;
 			}"
@@ -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__/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(`

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(`

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(`

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(`

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,19 +84,34 @@ 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';
 				declare const element: JSX.Element;
 				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<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,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;
 		});
 	}
 

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