Skip to content

Commit a62832e

Browse files
authored
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
1 parent e928761 commit a62832e

23 files changed

Lines changed: 862 additions & 787 deletions

.changeset/chilled-oranges-sit.md

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,15 @@
44

55
Ensure added imports of types use the `type` modifier
66

7-
If we'd previously add an import to `ReactNode` (e.g. in `deprecated-react-fragment`),
7+
If we'd previously add an import to `JSX` (e.g. in `scoped-jsx`),
88
the codemod would import it as a value.
99
This breaks TypeScript projects using `verbatimModuleSyntax` as well as projects enforcing `type` imports for types.
1010

1111
Now we ensure new imports of types use the `type` modifier:
1212

1313
```diff
14-
-import { ReactNode } from 'react'
15-
+import { type ReactNode } from 'react'
14+
-import { JSX } from 'react'
15+
+import { type JSX } from 'react'
1616
```
1717

1818
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.
4141
For example, the `deprecated-vfc-codemod` rewrites `VFC` identifiers to `FC`.
4242
If the import of `VFC` had no `type` modifier, the codemod will not add one.
4343

44-
This affects the following codemods:
45-
46-
- `deprecated-react-fragment`
47-
- `deprecated-react-node-array`
48-
- `scoped-jsx`
49-
5044
`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.

.changeset/famous-nails-destroy.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
"types-react-codemod": patch
3+
---
4+
5+
Ensure replace and rename codemods have consistent behavior
6+
7+
Fixes multiple incorrect transform patterns that were supported by some transforms but not others.
8+
We no longer switch to `type` imports if the original type wasn't imported with that modifier.
9+
Type parameters are now consistently preserved.
10+
We don't add a reference to the `React` namespace anymore if we can just add a type import.
11+
12+
This affects the following codemods:
13+
14+
- `deprecated-legacy-ref`
15+
- `deprecated-react-child`
16+
- `deprecated-react-text`
17+
- `deprecated-react-type`
18+
- `deprecated-sfc-element`
19+
- `deprecated-sfc`
20+
- `deprecated-stateless-component`
21+
- `deprecated-void-function-component`

.changeset/short-zoos-type.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,5 @@ Now we properly detect that e.g. `JSX` is used in `someFunctionWithTypeParameter
99
Affected codemods:
1010

1111
- `deprecated-react-child`
12-
- `deprecated-react-fragment`
13-
- `deprecated-react-node-array`
1412
- `deprecated-react-text`
1513
- `scoped-jsx`
Lines changed: 59 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
const { describe, expect, test } = require("@jest/globals");
1+
const { expect, test } = require("@jest/globals");
22
const dedent = require("dedent");
33
const JscodeshiftTestUtils = require("jscodeshift/dist/testUtils");
44
const deprecatedReactChildTransform = require("../deprecated-react-child");
@@ -14,80 +14,110 @@ function applyTransform(source, options = {}) {
1414
);
1515
}
1616

17-
describe("transform deprecated-react-child", () => {
18-
test("not modified", () => {
19-
expect(
20-
applyTransform(`
17+
test("not modified", () => {
18+
expect(
19+
applyTransform(`
2120
import * as React from 'react';
2221
interface Props {
2322
children?: ReactNode;
2423
}
2524
`),
26-
).toMatchInlineSnapshot(`
25+
).toMatchInlineSnapshot(`
2726
"import * as React from 'react';
2827
interface Props {
2928
children?: ReactNode;
3029
}"
3130
`);
32-
});
31+
});
3332

34-
test("named import", () => {
35-
expect(
36-
applyTransform(`
33+
test("named import", () => {
34+
expect(
35+
applyTransform(`
3736
import { ReactChild } from 'react';
3837
interface Props {
3938
children?: ReactChild;
4039
}
4140
`),
42-
).toMatchInlineSnapshot(`
43-
"import { ReactChild } from 'react';
41+
).toMatchInlineSnapshot(`
42+
"import { ReactElement } from 'react';
43+
interface Props {
44+
children?: ReactElement | number | string;
45+
}"
46+
`);
47+
});
48+
49+
test("named type import", () => {
50+
expect(
51+
applyTransform(`
52+
import { type ReactChild } from 'react';
53+
interface Props {
54+
children?: ReactChild;
55+
}
56+
`),
57+
).toMatchInlineSnapshot(`
58+
"import { type ReactElement } from 'react';
59+
interface Props {
60+
children?: ReactElement | number | string;
61+
}"
62+
`);
63+
});
64+
65+
test("named type import with existing target import", () => {
66+
expect(
67+
applyTransform(`
68+
import { ReactChild, ReactElement } from 'react';
69+
interface Props {
70+
children?: ReactChild;
71+
}
72+
`),
73+
).toMatchInlineSnapshot(`
74+
"import { ReactElement } from 'react';
4475
interface Props {
45-
children?: React.ReactElement | number | string;
76+
children?: ReactElement | number | string;
4677
}"
4778
`);
48-
});
79+
});
4980

50-
test("false-negative named renamed import", () => {
51-
expect(
52-
applyTransform(`
81+
test("false-negative named renamed import", () => {
82+
expect(
83+
applyTransform(`
5384
import { ReactChild as MyReactChild } from 'react';
5485
interface Props {
5586
children?: MyReactChild;
5687
}
5788
`),
58-
).toMatchInlineSnapshot(`
89+
).toMatchInlineSnapshot(`
5990
"import { ReactChild as MyReactChild } from 'react';
6091
interface Props {
6192
children?: MyReactChild;
6293
}"
6394
`);
64-
});
95+
});
6596

66-
test("namespace import", () => {
67-
expect(
68-
applyTransform(`
97+
test("namespace import", () => {
98+
expect(
99+
applyTransform(`
69100
import * as React from 'react';
70101
interface Props {
71102
children?: React.ReactChild;
72103
}
73104
`),
74-
).toMatchInlineSnapshot(`
105+
).toMatchInlineSnapshot(`
75106
"import * as React from 'react';
76107
interface Props {
77108
children?: React.ReactElement | number | string;
78109
}"
79110
`);
80-
});
111+
});
81112

82-
test("as type parameter", () => {
83-
expect(
84-
applyTransform(`
113+
test("as type parameter", () => {
114+
expect(
115+
applyTransform(`
85116
import * as React from 'react';
86117
createAction<React.ReactChild>()
87118
`),
88-
).toMatchInlineSnapshot(`
119+
).toMatchInlineSnapshot(`
89120
"import * as React from 'react';
90121
createAction<React.ReactElement | number | string>()"
91122
`);
92-
});
93123
});
Lines changed: 43 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
const { describe, expect, test } = require("@jest/globals");
1+
const { expect, test } = require("@jest/globals");
22
const dedent = require("dedent");
33
const JscodeshiftTestUtils = require("jscodeshift/dist/testUtils");
44
const deprecatedReactFragmentTransform = require("../deprecated-react-fragment");
@@ -14,128 +14,126 @@ function applyTransform(source, options = {}) {
1414
);
1515
}
1616

17-
describe("transform deprecated-react-node-array", () => {
18-
test("not modified", () => {
19-
expect(
20-
applyTransform(`
17+
test("not modified", () => {
18+
expect(
19+
applyTransform(`
2120
import * as React from 'react';
2221
interface Props {
2322
children?: ReactNode;
2423
}
2524
`),
26-
).toMatchInlineSnapshot(`
25+
).toMatchInlineSnapshot(`
2726
"import * as React from 'react';
2827
interface Props {
2928
children?: ReactNode;
3029
}"
3130
`);
32-
});
31+
});
3332

34-
test("named import", () => {
35-
expect(
36-
applyTransform(`
33+
test("named import", () => {
34+
expect(
35+
applyTransform(`
3736
import { ReactFragment } from 'react';
3837
interface Props {
3938
children?: ReactFragment;
4039
}
4140
`),
42-
).toMatchInlineSnapshot(`
43-
"import { type ReactNode } from 'react';
41+
).toMatchInlineSnapshot(`
42+
"import { ReactNode } from 'react';
4443
interface Props {
4544
children?: Iterable<ReactNode>;
4645
}"
4746
`);
48-
});
47+
});
4948

50-
test("named type import", () => {
51-
expect(
52-
applyTransform(`
53-
import type { ReactFragment } from 'react';
49+
test("named type import", () => {
50+
expect(
51+
applyTransform(`
52+
import { type ReactFragment } from 'react';
5453
interface Props {
5554
children?: ReactFragment;
5655
}
5756
`),
58-
).toMatchInlineSnapshot(`
59-
"import type { ReactNode } from 'react';
57+
).toMatchInlineSnapshot(`
58+
"import { type ReactNode } from 'react';
6059
interface Props {
6160
children?: Iterable<ReactNode>;
6261
}"
6362
`);
64-
});
63+
});
6564

66-
test("named import with existing ReactNode import", () => {
67-
expect(
68-
applyTransform(`
65+
test("named import with existing target import", () => {
66+
expect(
67+
applyTransform(`
6968
import { ReactFragment, ReactNode } from 'react';
7069
interface Props {
7170
children?: ReactFragment;
7271
}
7372
`),
74-
).toMatchInlineSnapshot(`
73+
).toMatchInlineSnapshot(`
7574
"import { ReactNode } from 'react';
7675
interface Props {
7776
children?: Iterable<ReactNode>;
7877
}"
7978
`);
80-
});
79+
});
8180

82-
test("named import with existing ReactNode type import", () => {
83-
expect(
84-
applyTransform(`
81+
test("named import with existing ReactNode type import", () => {
82+
expect(
83+
applyTransform(`
8584
import { ReactFragment, type ReactNode } from 'react';
8685
interface Props {
8786
children?: ReactFragment;
8887
}
8988
`),
90-
).toMatchInlineSnapshot(`
89+
).toMatchInlineSnapshot(`
9190
"import { type ReactNode } from 'react';
9291
interface Props {
9392
children?: Iterable<ReactNode>;
9493
}"
9594
`);
96-
});
95+
});
9796

98-
test("false-negative named renamed import", () => {
99-
expect(
100-
applyTransform(`
97+
test("false-negative named renamed import", () => {
98+
expect(
99+
applyTransform(`
101100
import { ReactFragment as MyReactFragment } from 'react';
102101
interface Props {
103102
children?: MyReactFragment;
104103
}
105104
`),
106-
).toMatchInlineSnapshot(`
105+
).toMatchInlineSnapshot(`
107106
"import { ReactFragment as MyReactFragment } from 'react';
108107
interface Props {
109108
children?: MyReactFragment;
110109
}"
111110
`);
112-
});
111+
});
113112

114-
test("namespace import", () => {
115-
expect(
116-
applyTransform(`
113+
test("namespace import", () => {
114+
expect(
115+
applyTransform(`
117116
import * as React from 'react';
118117
interface Props {
119118
children?: React.ReactFragment;
120119
}
121120
`),
122-
).toMatchInlineSnapshot(`
121+
).toMatchInlineSnapshot(`
123122
"import * as React from 'react';
124123
interface Props {
125124
children?: Iterable<React.ReactNode>;
126125
}"
127126
`);
128-
});
127+
});
129128

130-
test("as type parameter", () => {
131-
expect(
132-
applyTransform(`
129+
test("as type parameter", () => {
130+
expect(
131+
applyTransform(`
133132
import * as React from 'react';
134133
createComponent<React.ReactFragment>();
135134
`),
136-
).toMatchInlineSnapshot(`
135+
).toMatchInlineSnapshot(`
137136
"import * as React from 'react';
138137
createComponent<Iterable<React.ReactNode>>();"
139138
`);
140-
});
141139
});

0 commit comments

Comments
 (0)