Skip to content

Commit f05624f

Browse files
eps1loneps1lon
authored
fix: Ensure added imports of types use the type modifier (#343)
1 parent c417265 commit f05624f

12 files changed

Lines changed: 294 additions & 57 deletions

‎.changeset/chilled-oranges-sit.md‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
"types-react-codemod": patch
3+
---
4+
5+
Ensure added imports of types use the `type` modifier
6+
7+
If we'd previously add an import to `ReactNode` (e.g. in `deprecated-react-fragment`),
8+
the codemod would import it as a value.
9+
This breaks TypeScript projects using `verbatimModuleSyntax` as well as projects enforcing `type` imports for types.
10+
11+
Now we ensure new imports of types use the `type` modifier:
12+
13+
```diff
14+
-import { ReactNode } from 'react'
15+
+import { type ReactNode } from 'react'
16+
```
17+
18+
This also changes how we transform the deprecated global JSX namespace.
19+
Instead of rewriting each usage, we opt for adding another import.
20+
The guiding principle being that we keep the changes minimal in a codemod.
21+
22+
Before:
23+
24+
```diff
25+
import * as React from 'react'
26+
27+
-const element: JSX.Element
28+
+const element: React.JSX.Element
29+
```
30+
31+
After:
32+
33+
```diff
34+
import * as React from 'react'
35+
+import { type JSX } from 'react'
36+
37+
const element: JSX.Element
38+
```
39+
40+
Note that rewriting of imports does not change the modifier.
41+
For example, the `deprecated-vfc-codemod` rewrites `VFC` identifiers to `FC`.
42+
If the import of `VFC` had no `type` modifier, the codemod will not add one.
43+
44+
This affects the following codemods:
45+
46+
- `deprecated-react-fragment`
47+
- `deprecated-react-node-array`
48+
- `scoped-jsx`
49+
50+
`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‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,23 @@ describe("transform deprecated-react-node-array", () => {
4040
}
4141
`),
4242
).toMatchInlineSnapshot(`
43-
"import { ReactNode } from 'react';
43+
"import { type ReactNode } from 'react';
44+
interface Props {
45+
children?: Iterable<ReactNode>;
46+
}"
47+
`);
48+
});
49+
50+
test("named type import", () => {
51+
expect(
52+
applyTransform(`
53+
import type { ReactFragment } from 'react';
54+
interface Props {
55+
children?: ReactFragment;
56+
}
57+
`),
58+
).toMatchInlineSnapshot(`
59+
"import type { ReactNode } from 'react';
4460
interface Props {
4561
children?: Iterable<ReactNode>;
4662
}"
@@ -63,6 +79,22 @@ describe("transform deprecated-react-node-array", () => {
6379
`);
6480
});
6581

82+
test("named import with existing ReactNode type import", () => {
83+
expect(
84+
applyTransform(`
85+
import { ReactFragment, type ReactNode } from 'react';
86+
interface Props {
87+
children?: ReactFragment;
88+
}
89+
`),
90+
).toMatchInlineSnapshot(`
91+
"import { type ReactNode } from 'react';
92+
interface Props {
93+
children?: Iterable<ReactNode>;
94+
}"
95+
`);
96+
});
97+
6698
test("false-negative named renamed import", () => {
6799
expect(
68100
applyTransform(`

‎transforms/__tests__/deprecated-react-node-array.js‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,23 @@ describe("transform deprecated-react-node-array", () => {
4040
}
4141
`),
4242
).toMatchInlineSnapshot(`
43-
"import { ReactNode } from 'react';
43+
"import { type ReactNode } from 'react';
44+
interface Props {
45+
children?: ReadonlyArray<ReactNode>;
46+
}"
47+
`);
48+
});
49+
50+
test("named type import", () => {
51+
expect(
52+
applyTransform(`
53+
import type { ReactNodeArray } from 'react';
54+
interface Props {
55+
children?: ReactNodeArray;
56+
}
57+
`),
58+
).toMatchInlineSnapshot(`
59+
"import type { ReactNode } from 'react';
4460
interface Props {
4561
children?: ReadonlyArray<ReactNode>;
4662
}"
@@ -63,6 +79,22 @@ describe("transform deprecated-react-node-array", () => {
6379
`);
6480
});
6581

82+
test("named import with existing ReactNode type import", () => {
83+
expect(
84+
applyTransform(`
85+
import { ReactNodeArray, type ReactNode } from 'react';
86+
interface Props {
87+
children?: ReactNodeArray;
88+
}
89+
`),
90+
).toMatchInlineSnapshot(`
91+
"import { type ReactNode } from 'react';
92+
interface Props {
93+
children?: ReadonlyArray<ReactNode>;
94+
}"
95+
`);
96+
});
97+
6698
test("false-negative named renamed import", () => {
6799
expect(
68100
applyTransform(`

‎transforms/__tests__/deprecated-sfc-element.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,18 @@ describe("transform deprecated-sfc-element", () => {
3939
`);
4040
});
4141

42+
test("named typew import", () => {
43+
expect(
44+
applyTransform(`
45+
import { type SFCElement } from 'react';
46+
SFCElement;
47+
`),
48+
).toMatchInlineSnapshot(`
49+
"import { type FunctionComponentElement } from 'react';
50+
FunctionComponentElement;"
51+
`);
52+
});
53+
4254
test("named renamed import", () => {
4355
expect(
4456
applyTransform(`

‎transforms/__tests__/deprecated-sfc.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,18 @@ describe("transform deprecated-sfc", () => {
3535
`);
3636
});
3737

38+
test("named type import", () => {
39+
expect(
40+
applyTransform(`
41+
import { type SFC } from 'react';
42+
SFC;
43+
`),
44+
).toMatchInlineSnapshot(`
45+
"import { type FC } from 'react';
46+
FC;"
47+
`);
48+
});
49+
3850
test("named renamed import", () => {
3951
expect(
4052
applyTransform(`

‎transforms/__tests__/deprecated-stateless-component.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,18 @@ describe("transform deprecated-stateless-component", () => {
3939
`);
4040
});
4141

42+
test("named type import", () => {
43+
expect(
44+
applyTransform(`
45+
import { type StatelessComponent } from 'react';
46+
StatelessComponent;
47+
`),
48+
).toMatchInlineSnapshot(`
49+
"import { type FunctionComponent } from 'react';
50+
FunctionComponent;"
51+
`);
52+
});
53+
4254
test("named renamed import", () => {
4355
expect(
4456
applyTransform(`

‎transforms/__tests__/deprecated-void-function-component.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,20 @@ describe("transform deprecated-void-function-component", () => {
4141
`);
4242
});
4343

44+
test("named type import", () => {
45+
expect(
46+
applyTransform(`
47+
import { type VFC, type VoidFunctionComponent } from 'react';
48+
VFC;
49+
VoidFunctionComponent;
50+
`),
51+
).toMatchInlineSnapshot(`
52+
"import { type FC, type FunctionComponent } from 'react';
53+
FC;
54+
FunctionComponent;"
55+
`);
56+
});
57+
4458
test("named renamed import", () => {
4559
expect(
4660
applyTransform(`

‎transforms/__tests__/scoped-jsx.js‎

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,32 @@ describe("transform scoped-jsx", () => {
3333
declare const element: JSX.Element;
3434
`),
3535
).toMatchInlineSnapshot(`
36-
"import { JSX } from "react";
36+
"import type { JSX } from "react";
37+
declare const element: JSX.Element;"
38+
`);
39+
});
40+
41+
test("existing default import", () => {
42+
expect(
43+
applyTransform(`
44+
import React from 'react';
45+
declare const element: JSX.Element;
46+
`),
47+
).toMatchInlineSnapshot(`
48+
"import React, { type JSX } from 'react';
49+
declare const element: JSX.Element;"
50+
`);
51+
});
52+
53+
test("existing type default import", () => {
54+
expect(
55+
applyTransform(`
56+
import type React from 'react';
57+
declare const element: JSX.Element;
58+
`),
59+
).toMatchInlineSnapshot(`
60+
"import type React from 'react';
61+
import type { JSX } from "react";
3762
declare const element: JSX.Element;"
3863
`);
3964
});
@@ -46,7 +71,8 @@ describe("transform scoped-jsx", () => {
4671
`),
4772
).toMatchInlineSnapshot(`
4873
"import * as React from 'react';
49-
declare const element: React.JSX.Element;"
74+
import type { JSX } from "react";
75+
declare const element: JSX.Element;"
5076
`);
5177
});
5278

@@ -58,19 +84,34 @@ describe("transform scoped-jsx", () => {
5884
`),
5985
).toMatchInlineSnapshot(`
6086
"import type * as React from 'react';
61-
declare const element: React.JSX.Element;"
87+
import type { JSX } from "react";
88+
declare const element: JSX.Element;"
6289
`);
6390
});
6491

65-
test("existing named import", () => {
92+
test("existing named import with other named imports", () => {
6693
expect(
6794
applyTransform(`
6895
import { ReactNode } from 'react';
6996
declare const element: JSX.Element;
7097
declare const node: ReactNode;
7198
`),
7299
).toMatchInlineSnapshot(`
73-
"import { ReactNode, JSX } from 'react';
100+
"import { ReactNode, type JSX } from 'react';
101+
declare const element: JSX.Element;
102+
declare const node: ReactNode;"
103+
`);
104+
});
105+
106+
test("existing named import with other named type imports", () => {
107+
expect(
108+
applyTransform(`
109+
import type { ReactNode } from 'react';
110+
declare const element: JSX.Element;
111+
declare const node: ReactNode;
112+
`),
113+
).toMatchInlineSnapshot(`
114+
"import type { ReactNode, JSX } from 'react';
74115
declare const element: JSX.Element;
75116
declare const node: ReactNode;"
76117
`);
@@ -95,7 +136,7 @@ describe("transform scoped-jsx", () => {
95136
declare const element: JSX.Element;
96137
`),
97138
).toMatchInlineSnapshot(`
98-
"import { JSX } from "react";
139+
"import type { JSX } from "react";
99140
const React = require('react');
100141
declare const element: JSX.Element;"
101142
`);
@@ -113,7 +154,7 @@ describe("transform scoped-jsx", () => {
113154
"import {} from 'react-dom'
114155
import {} from '@testing-library/react'
115156
116-
import { JSX } from "react";
157+
import type { JSX } from "react";
117158
118159
declare const element: JSX.Element;"
119160
`);
@@ -129,7 +170,9 @@ describe("transform scoped-jsx", () => {
129170
).toMatchInlineSnapshot(`
130171
"import * as React from 'react'
131172
132-
declare const attributes: React.JSX.LibraryManagedAttributes<A, B>;"
173+
import type { JSX } from "react";
174+
175+
declare const attributes: JSX.LibraryManagedAttributes<A, B>;"
133176
`);
134177
});
135178
});

‎transforms/deprecated-react-fragment.js‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
const parseSync = require("./utils/parseSync");
2-
32
/**
43
* @type {import('jscodeshift').Transform}
54
*/
@@ -29,8 +28,15 @@ const deprecatedReactFragmentTransform = (file, api) => {
2928
if (hasReactNodeImport.length > 0) {
3029
reactFragmentImports.remove();
3130
} else {
32-
reactFragmentImports.replaceWith(() => {
33-
return j.importSpecifier(j.identifier("ReactNode"));
31+
reactFragmentImports.replaceWith((path) => {
32+
const importSpecifier = j.importSpecifier(j.identifier("ReactNode"));
33+
const importDeclaration = path.parentPath.parentPath.value;
34+
if (importDeclaration.importKind !== "type") {
35+
// @ts-expect-error -- Missing types in jscodeshift. Babel uses `importKind`: https://astexplorer.net/#/gist/a76bd35f28483a467fef29d3c63aac9b/0e7ba6688fc09bd11b92197349b2384bb4c94574
36+
importSpecifier.importKind = "type";
37+
}
38+
39+
return importSpecifier;
3440
});
3541
}
3642

‎transforms/deprecated-react-node-array.js‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,16 @@ const deprecatedReactNodeArrayTransform = (file, api) => {
2929
if (hasReactNodeImport.length > 0) {
3030
reactNodeArrayImports.remove();
3131
} else {
32-
reactNodeArrayImports.replaceWith(() => {
33-
return j.importSpecifier(j.identifier("ReactNode"));
32+
reactNodeArrayImports.replaceWith((path) => {
33+
const importSpecifier = j.importSpecifier(j.identifier("ReactNode"));
34+
35+
const importDeclaration = path.parentPath.parentPath.value;
36+
if (importDeclaration.importKind !== "type") {
37+
// @ts-expect-error -- Missing types in jscodeshift. Babel uses `importKind`: https://astexplorer.net/#/gist/a76bd35f28483a467fef29d3c63aac9b/0e7ba6688fc09bd11b92197349b2384bb4c94574
38+
importSpecifier.importKind = "type";
39+
}
40+
41+
return importSpecifier;
3442
});
3543
}
3644

0 commit comments

Comments
 (0)