fix: Consistent behavior for rename and replace transforms (#348)

* Codify current behavior for all transforms targetting deprecated types * fix: Consistent behavior for rename and replace transforms
eps1lon · Feb 19, 2024 · a62832e · a62832e
1 parent e928761
commit a62832e
Show file tree

Hide file tree

Showing 23 changed files with 862 additions and 787 deletions.
diff --git a/.changeset/chilled-oranges-sit.md b/.changeset/chilled-oranges-sit.md
@@ -4,15 +4,15 @@
 
 Ensure added imports of types use the `type` modifier
 
-If we'd previously add an import to `ReactNode` (e.g. in `deprecated-react-fragment`),
+If we'd previously add an import to `JSX` (e.g. in `scoped-jsx`),
 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'
+-import { JSX } from 'react'
++import { type JSX } from 'react'
 ```
 
 This also changes how we transform the deprecated global JSX namespace.
@@ -41,10 +41,4 @@ 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/.changeset/famous-nails-destroy.md b/.changeset/famous-nails-destroy.md
@@ -0,0 +1,21 @@
+---
+"types-react-codemod": patch
+---
+
+Ensure replace and rename codemods have consistent behavior
+
+Fixes multiple incorrect transform patterns that were supported by some transforms but not others.
+We no longer switch to `type` imports if the original type wasn't imported with that modifier.
+Type parameters are now consistently preserved.
+We don't add a reference to the `React` namespace anymore if we can just add a type import.
+
+This affects the following codemods:
+
+- `deprecated-legacy-ref`
+- `deprecated-react-child`
+- `deprecated-react-text`
+- `deprecated-react-type`
+- `deprecated-sfc-element`
+- `deprecated-sfc`
+- `deprecated-stateless-component`
+- `deprecated-void-function-component`
diff --git a/.changeset/short-zoos-type.md b/.changeset/short-zoos-type.md
@@ -9,7 +9,5 @@ Now we properly detect that e.g. `JSX` is used in `someFunctionWithTypeParameter
 Affected codemods:
 
 - `deprecated-react-child`
-- `deprecated-react-fragment`
-- `deprecated-react-node-array`
 - `deprecated-react-text`
 - `scoped-jsx`
diff --git a/transforms/__tests__/deprecated-react-child.js b/transforms/__tests__/deprecated-react-child.js
@@ -1,4 +1,4 @@
-const { describe, expect, test } = require("@jest/globals");
+const { expect, test } = require("@jest/globals");
 const dedent = require("dedent");
 const JscodeshiftTestUtils = require("jscodeshift/dist/testUtils");
 const deprecatedReactChildTransform = require("../deprecated-react-child");
@@ -14,80 +14,110 @@ function applyTransform(source, options = {}) {
 	);
 }
 
-describe("transform deprecated-react-child", () => {
-	test("not modified", () => {
-		expect(
-			applyTransform(`
+test("not modified", () => {
+	expect(
+		applyTransform(`
 			import * as React from 'react';
 			interface Props {
 				children?: ReactNode;
 			}
     `),
-		).toMatchInlineSnapshot(`
+	).toMatchInlineSnapshot(`
 		"import * as React from 'react';
 		interface Props {
 			children?: ReactNode;
 		}"
 	`);
-	});
+});
 
-	test("named import", () => {
-		expect(
-			applyTransform(`
+test("named import", () => {
+	expect(
+		applyTransform(`
       import { ReactChild } from 'react';
 		interface Props {
 				children?: ReactChild;
 			}
     `),
-		).toMatchInlineSnapshot(`
-		"import { ReactChild } from 'react';
+	).toMatchInlineSnapshot(`
+		"import { ReactElement } from 'react';
+		interface Props {
+				children?: ReactElement | number | string;
+			}"
+	`);
+});
+
+test("named type import", () => {
+	expect(
+		applyTransform(`
+      import { type ReactChild } from 'react';
+		interface Props {
+				children?: ReactChild;
+			}
+    `),
+	).toMatchInlineSnapshot(`
+		"import { type ReactElement } from 'react';
+		interface Props {
+				children?: ReactElement | number | string;
+			}"
+	`);
+});
+
+test("named type import with existing target import", () => {
+	expect(
+		applyTransform(`
+      import { ReactChild, ReactElement } from 'react';
+		interface Props {
+				children?: ReactChild;
+			}
+    `),
+	).toMatchInlineSnapshot(`
+		"import { ReactElement } from 'react';
 		interface Props {
-				children?: React.ReactElement | number | string;
+				children?: ReactElement | number | string;
 			}"
 	`);
-	});
+});
 
-	test("false-negative named renamed import", () => {
-		expect(
-			applyTransform(`
+test("false-negative named renamed import", () => {
+	expect(
+		applyTransform(`
       import { ReactChild as MyReactChild } from 'react';
       interface Props {
 				children?: MyReactChild;
 			}
     `),
-		).toMatchInlineSnapshot(`
+	).toMatchInlineSnapshot(`
 		"import { ReactChild as MyReactChild } from 'react';
 		   interface Props {
 			children?: MyReactChild;
 		}"
 	`);
-	});
+});
 
-	test("namespace import", () => {
-		expect(
-			applyTransform(`
+test("namespace import", () => {
+	expect(
+		applyTransform(`
       import * as React from 'react';
       interface Props {
 				children?: React.ReactChild;
 			}
     `),
-		).toMatchInlineSnapshot(`
+	).toMatchInlineSnapshot(`
 		"import * as React from 'react';
 		   interface Props {
 			children?: React.ReactElement | number | string;
 		}"
 	`);
-	});
+});
 
-	test("as type parameter", () => {
-		expect(
-			applyTransform(`
+test("as type parameter", () => {
+	expect(
+		applyTransform(`
 			import * as React from 'react';
 			createAction<React.ReactChild>()
 		`),
-		).toMatchInlineSnapshot(`
+	).toMatchInlineSnapshot(`
 		"import * as React from 'react';
 		createAction<React.ReactElement | number | string>()"
 	`);
-	});
 });
diff --git a/transforms/__tests__/deprecated-react-fragment.js b/transforms/__tests__/deprecated-react-fragment.js
@@ -1,4 +1,4 @@
-const { describe, expect, test } = require("@jest/globals");
+const { expect, test } = require("@jest/globals");
 const dedent = require("dedent");
 const JscodeshiftTestUtils = require("jscodeshift/dist/testUtils");
 const deprecatedReactFragmentTransform = require("../deprecated-react-fragment");
@@ -14,128 +14,126 @@ function applyTransform(source, options = {}) {
 	);
 }
 
-describe("transform deprecated-react-node-array", () => {
-	test("not modified", () => {
-		expect(
-			applyTransform(`
+test("not modified", () => {
+	expect(
+		applyTransform(`
 			import * as React from 'react';
 			interface Props {
 				children?: ReactNode;
 			}
     `),
-		).toMatchInlineSnapshot(`
+	).toMatchInlineSnapshot(`
 		"import * as React from 'react';
 		interface Props {
 			children?: ReactNode;
 		}"
 	`);
-	});
+});
 
-	test("named import", () => {
-		expect(
-			applyTransform(`
+test("named import", () => {
+	expect(
+		applyTransform(`
       import { ReactFragment } from 'react';
 		interface Props {
 				children?: ReactFragment;
 			}
     `),
-		).toMatchInlineSnapshot(`
-		"import { type ReactNode } from 'react';
+	).toMatchInlineSnapshot(`
+		"import { ReactNode } from 'react';
 		interface Props {
 				children?: Iterable<ReactNode>;
 			}"
 	`);
-	});
+});
 
-	test("named type import", () => {
-		expect(
-			applyTransform(`
-      import type { ReactFragment } from 'react';
+test("named type import", () => {
+	expect(
+		applyTransform(`
+      import { type ReactFragment } from 'react';
 		interface Props {
 				children?: ReactFragment;
 			}
     `),
-		).toMatchInlineSnapshot(`
-		"import type { ReactNode } from 'react';
+	).toMatchInlineSnapshot(`
+		"import { type ReactNode } from 'react';
 		interface Props {
 				children?: Iterable<ReactNode>;
 			}"
 	`);
-	});
+});
 
-	test("named import with existing ReactNode import", () => {
-		expect(
-			applyTransform(`
+test("named import with existing target import", () => {
+	expect(
+		applyTransform(`
       import { ReactFragment, ReactNode } from 'react';
 		interface Props {
 				children?: ReactFragment;
 			}
     `),
-		).toMatchInlineSnapshot(`
+	).toMatchInlineSnapshot(`
 		"import { ReactNode } from 'react';
 		interface Props {
 				children?: Iterable<ReactNode>;
 			}"
 	`);
-	});
+});
 
-	test("named import with existing ReactNode type import", () => {
-		expect(
-			applyTransform(`
+test("named import with existing ReactNode type import", () => {
+	expect(
+		applyTransform(`
       import { ReactFragment, type ReactNode } from 'react';
 		interface Props {
 				children?: ReactFragment;
 			}
     `),
-		).toMatchInlineSnapshot(`
+	).toMatchInlineSnapshot(`
 		"import { type ReactNode } from 'react';
 		interface Props {
 				children?: Iterable<ReactNode>;
 			}"
 	`);
-	});
+});
 
-	test("false-negative named renamed import", () => {
-		expect(
-			applyTransform(`
+test("false-negative named renamed import", () => {
+	expect(
+		applyTransform(`
       import { ReactFragment as MyReactFragment } from 'react';
       interface Props {
 				children?: MyReactFragment;
 			}
     `),
-		).toMatchInlineSnapshot(`
+	).toMatchInlineSnapshot(`
 		"import { ReactFragment as MyReactFragment } from 'react';
 		   interface Props {
 			children?: MyReactFragment;
 		}"
 	`);
-	});
+});
 
-	test("namespace import", () => {
-		expect(
-			applyTransform(`
+test("namespace import", () => {
+	expect(
+		applyTransform(`
       import * as React from 'react';
       interface Props {
 				children?: React.ReactFragment;
 			}
     `),
-		).toMatchInlineSnapshot(`
+	).toMatchInlineSnapshot(`
 		"import * as React from 'react';
 		   interface Props {
 			children?: Iterable<React.ReactNode>;
 		}"
 	`);
-	});
+});
 
-	test("as type parameter", () => {
-		expect(
-			applyTransform(`
+test("as type parameter", () => {
+	expect(
+		applyTransform(`
       import * as React from 'react';
       createComponent<React.ReactFragment>();
     `),
-		).toMatchInlineSnapshot(`
+	).toMatchInlineSnapshot(`
 		"import * as React from 'react';
 		createComponent<Iterable<React.ReactNode>>();"
 	`);
-	});
 });