Skip to content

Commit 8e1675e

Browse files
committed
[react] improve handling of React types
1. Handle named imports In addition to supporting `React.*` types: ```js import * as React from 'react'; type T = React.Node | React.Element | React.ChildrenArray; ``` We also support named imports: ```js import { type Node as ReactNode, type Element as ReactElement, type ChildrenArray } from 'react'; type T = ReactNode | ReactElement | ChildrenArray; ``` NOTE: To avoid name conflicts with the DOM `Node` and `Element` type, it is important to alias `React.Node` as `ReactNode` and `React.Element` as `ReactElement`. 2. Strip type annotations from React function component return types in favor of inference Flow: ```js const App = ({ message }: AppProps): React.Node => <div>{message}</div>; ``` TypeScript: ```ts const App = ({ message }: AppProps) => <div>{message}</div>; ``` Why not simply change `React.Node` to `React.ReactNode`? If we leave it as `React.ReactNode`, we'll receive the error `'Component' cannot be used as a JSX component`. To address this, we simply strip out the type annotation from the return type of the function component and allow TypeScript to infer it. 3. Strip out `AbstractComponent` in favor of inference Flow: ```js // @flow const C: React.AbstractComponent<Props, mixed> = React.forwardRef<Props, Ref>(Comp); export default (forwardRef<Props, Ref>(Comp): React.AbstractComponent<Props, mixed>); ``` TypeScript: ```ts const C = forwardRef<Ref, Props>(Comp); export default forwardRef<Ref, Props>(Comp); ``` Why? Because there is no `React.AbstractComponent` equivalent in TypeScript. We can simply strip it out and allow TypeScript to infer the type. 4. Reverse params for `forwardRef` Flow: ```js forwardRef<Props, Ref>(Comp); ``` TypeScript: ```ts forwardRef<Ref, Props>(Comp); ``` Why? Because the arguments are swapped in TypeScript. 5. Rename `ElementConfig`, `ElementProps`, `Portal`, and `StatelessFunctionalComponent` - `ElementConfig` --> `ComponentProps` - `ElementProps` --> `ComponentProps` - `Portal` --> `PortalProps` - `StatelessFunctionalComponent` --> `FC`
1 parent 1977506 commit 8e1675e

12 files changed

+246
-90
lines changed

NOTES.md

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
# Notes
2-
## React.Node -> React.ReactElement
2+
## Strip React.Node from function component return types
33

44
One issue we encountered was most codemods convert `React.Node` to `React.ReactNode` when they're a function return type. While the names are similar, they behave [differently](https://stackoverflow.com/questions/58123398/when-to-use-jsx-element-vs-reactnode-vs-reactelement?rq=1). In TypeScript, `React.ReactNode` is primarily used to type things that can be children of a React node, and it includes things like booleans or null. When we return `React.Node`, we're usually annotating a functional component that can be instantiated in JSX (`<Component />`). In TypeScript that's inferred [as React.ReactElement or JSX.Element](https://github.com/TypeScript-cheatsheets/react#function-components). This also needs to be done for the `render` method of class components as well, despite some of the [React types](https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/react/index.d.ts#L3087) suggesting otherwise. If you leave it as `React.ReactNode` you'll receive the error `'Component' cannot be used as a JSX component`. In addition, if the function returns `null`, we have to add a `| null` to allow it. Strings and numbers will throw an error if returned in TS.
55

66
[TS Playground](https://www.TypeScriptlang.org/play?#code/JYWwDg9gTgLgBAJQKYEMDG8BmUIjgcilQ3wG4Aoc4AOxiSk3STgAUcwBnOAb3Ln7gcA-AC5BMKDQDmFAL6UkAD0iw4aCNQ7wAKki0BGOAF44ACgCUxgHw8+AojACuUanAA8AE2AA3K3S1uAPRevuTy5Eoq8OqaOnowAEzGZmDsHGJsEJyWRja8AnDAmClpAHQclvkF9khOLu4hVtypWRzlskGNdvzyBQ7OrtSOADbDYQrK0NEaWnC6WgDMyaYtnBlp5mLI6DCl2xgAchAezLm2BUUlreWV3dX99Z4+TattHB3Bz3e9NXWDI2NwpEpmoZnEtAAWZavdatTaIYi7fYwACiwyQICQtDgAB84ENRtZzgJLisyhVidU4A9XE9fM1yR8ugUfvwafiAXJKJQYrNMBAIMkAEIC9EoaimfTmCjkNDDFAcLjzGAAVjgSjo1A8XGRpQAwrhINQsfAqmysScoBYtoi9oi0RiTZTqqT+RBLOzTHS4IErNK7n1agMOaMKCy5DK5QqlfEAGzqxSa7UInb6w0aJ1m6kW+jWlMYO07B2Y7F4gnDZ0XYqmN0eoP1L0hH1+sNU7N-EPDVs9CM8sFwPUACyQaAA1sscnk7p6AwI3FZZ1S3MrDL7F9Vl-Ekmu2xvlUsd7u58qoYej+5lWqz0fN1p49eCkE-eNyEA)
77

8+
To address the issue, we remove the function component return type annotations and allow TypeScript to infer the return types.
9+
```ts
10+
const App = ({ message }: AppProps) => <div>{message}</div>;
11+
```
12+
813
## {\[KeyType\]: ValueType} -> Partial<Record<KeyType, ValueType>>
914
Object index types work pretty differently between Flow and TypeScript. Flow allows any type to be an index, while TypeScript supports only strings and numbers. One other difference I found is Flow marks all of the keys of the object as optional by default. To get this same behavior, we have to wrap the object in `Partial`.
1015

@@ -83,7 +88,7 @@ All of the types are imported and namespaced, and the codemod automatically adds
8388

8489
## React.AbstractComponent
8590

86-
[AbstractComponent](https://flow.org/en/docs/react/types/#toc-react-abstractcomponent) is a Flow type that helps you construct higher order components, and there is no direct TypeScript equivalent. The identical type is a `ComponentType` with the type of `ref` modified. That is a lot more verbose, so we added the `Flow.AbstractComponent` utility to keep things concise.
91+
[AbstractComponent](https://flow.org/en/docs/react/types/#toc-react-abstractcomponent) is a Flow type that helps you construct higher order components, and there is no direct TypeScript equivalent. We remove the annotations of `AbstractComponent` and allow TypeScript to infer them.
8792

8893
## export type * from './foo`
8994

@@ -94,11 +99,11 @@ In many cases, it's ideal to keep `type` imports when TypeScript supports it. Th
9499
We've encountered some global Flow types like [`TimeoutID`](https://github.com/facebook/flow/issues/5627) that needed conversions.
95100

96101

97-
## React.ElementConfig -> JSX.LibraryManagedAttributes
102+
## React.ElementConfig -> React.ComponentProps
98103

99-
Flow has a utility type which gets the props from a component. [TypeScript has a similar type](https://github.com/Microsoft/TypeScript/issues/26704) called `JSX.LibraryManagedAttributes` but there's some extra conversion needed to make the type parameters work.
104+
Flow has a utility type which gets the props from a component. TypeScript has a similar type called `ComponentProps`.
100105

101-
[TS Playground example](https://www.TypeScriptlang.org/play?ts=4.4.2#code/JYWwDg9gTgLgBAJQKYEMDG8BmUIjgcilQ3wG4BYAKCuADsYkpN0k4AFHMAZzgG8q4cGAE8ANowBccAEYQI4lLQqUAvlSppRKLjwAqSLvCQAPBrQAmPZOhgA6AMK5ItJPQA8HCNwB8fAXDQIWkMoAFcMaAAKME4uKU9uAEo-SkFBLlCwRmjYxOVBNWpUuCILbMSpawxbKpgAOQhzVn5iwSIYUKhaOEj-NLg3c2AANzgAem8+uDz-QsL1SjoGJhY4R3Ag13gWwRivLgB+KUVhZXnKESy4fUMEngBeOAApAGUADVsAGWBpKBQoYQAWUUKAA5khzABBGAwKA-UIMLhuG4wAA01wMMAA2gAiPbcHEAXW8ykurF0YkYcEeKLuWPwInEUHwhNIQA)
106+
[TS Playground example](https://www.typescriptlang.org/play?ts=4.4.2#code/JYWwDg9gTgLgBAJQKYEMDG8BmUIjgcilQ3wG4BYAKCuADsYkpN0k4AFHMAZzgG8q4cGAE8ANowBccAEYQI4lLQqUAvlSppRKLjwAqSLvCQAPBrQAmPZOhgA6AMK5ItJPQA8HCNwB8fAXDQIWkMoAFcMaAAKME4uKU9uAEo-SkFBLlCwRmjYxOVBNWpUuCILbMSpawxbKpgAOQhzVn5iwSIYUKhaOEj-NLg3c2AANzgAem8+uDz-QsL1SjoGJhY4R3Ag13gWwRivLgB+KUVhZXnKESy4fUMEngBeRGI7dectu7dLpAhMa4MYbzKL7XMSMOCPG4wO4AbXwInEUHwAF1SEA)
102107

103108
## Utility types
104109

src/convert/declarations.test.ts

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,12 @@ describe("transform declarations", () => {
107107
expectMigrationReporterMethodNotCalled(`importWithExtension`);
108108
});
109109

110+
it("drops React.AbstractComponent imports", async () => {
111+
const src = `import {type AbstractComponent, forwardRef} from 'react';`;
112+
const expected = `import {forwardRef} from 'react';`;
113+
expect(await transform(src)).toBe(expected);
114+
});
115+
110116
describe("Flow to TypeScript React import transformations", () => {
111117
Object.entries(ReactTypes).forEach(([flowType, tsType]) => {
112118
it(`transforms type imports of ${flowType} from react`, async () => {
@@ -561,43 +567,33 @@ describe("transform declarations", () => {
561567
expect(await transform(src)).toBe(expected);
562568
});
563569

564-
it("Converts React.Node to React.ReactElement in render", async () => {
565-
const src = dedent`class Foo extends React.Component {
566-
render(): React.Node {return <div />};
567-
};`;
568-
const expected = dedent`class Foo extends React.Component {
569-
render(): React.ReactElement {return <div />};
570-
};`;
571-
expect(await transform(src)).toBe(expected);
572-
});
573-
574-
it("Adds null to React.ReactElement in render", async () => {
570+
it("Converts React.Node to ReactNode in render", async () => {
575571
const src = dedent`class Foo extends React.Component {
576572
render(): React.Node {
577573
if (foo) return (<div />);
578574
return null;
579575
};
580576
};`;
581577
const expected = dedent`class Foo extends React.Component {
582-
render(): React.ReactElement | null {
578+
render(): ReactNode {
583579
if (foo) return (<div />);
584580
return null;
585581
};
586582
};`;
587583
expect(await transform(src)).toBe(expected);
588584
});
589585

590-
it("Converts React.Node to React.ReactElement for render in arrow", async () => {
586+
it("Strips out React.Node for render in arrow", async () => {
591587
const src = dedent`class Foo extends React.Component {
592588
render = (): React.Node => {return <div />};
593589
};`;
594590
const expected = dedent`class Foo extends React.Component {
595-
render = (): React.ReactElement => {return <div />};
591+
render = () => {return <div />};
596592
};`;
597593
expect(await transform(src)).toBe(expected);
598594
});
599595

600-
it("Does not convert React.Node to React.ReactElement in non-render", async () => {
596+
it("Does not convert React.Node to ReactNode in non-render", async () => {
601597
const src = dedent`class Foo extends React.Component {
602598
rendering(): React.Node {return <div />};
603599
};`;
@@ -607,6 +603,19 @@ describe("transform declarations", () => {
607603
expect(await transform(src)).toBe(expected);
608604
});
609605

606+
it("removes React.AbstractComponent<>", async () => {
607+
const src = dedent`
608+
// @flow
609+
const C1: AbstractComponent<Props, Ref> = memo<Props>((props) => null);
610+
const C2: AbstractComponent<Props, Ref> = memo<Props>(Comp);
611+
`;
612+
const expected = dedent`
613+
const C1 = memo<Props>((props) => null);
614+
const C2 = memo<Props>(Comp);
615+
`;
616+
expect(await transform(src)).toBe(expected);
617+
});
618+
610619
describe("untyped usestate", () => {
611620
it("Applies any and creates warning for untyped empty useState.", async () => {
612621
const src = `const test = React.useState();`;

src/convert/declarations.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,18 @@ export function transformDeclarations({
145145
}
146146
}
147147

148+
// `import {type AbstractComponent, ...} from 'react'` => `import {...} from 'react'`
149+
if (path.node.source.value === "react") {
150+
path.node.specifiers = path.node.specifiers.filter(
151+
(s) =>
152+
!(
153+
s.type === "ImportSpecifier" &&
154+
s.imported.type === "Identifier" &&
155+
s.imported.name === "AbstractComponent"
156+
)
157+
);
158+
}
159+
148160
return;
149161
}
150162

@@ -291,6 +303,19 @@ export function transformDeclarations({
291303
},
292304

293305
VariableDeclarator(path) {
306+
// `const c: AbstractComponent<Props, RefType> =` → `const c =`
307+
if (
308+
path.node.id.type === "Identifier" &&
309+
path.node.id.typeAnnotation?.type === "TypeAnnotation" &&
310+
path.node.id.typeAnnotation.typeAnnotation.type ===
311+
"GenericTypeAnnotation" &&
312+
path.node.id.typeAnnotation.typeAnnotation.id.type === "Identifier" &&
313+
path.node.id.typeAnnotation.typeAnnotation.id.name ===
314+
"AbstractComponent"
315+
) {
316+
path.node.id.typeAnnotation = null;
317+
}
318+
294319
if (
295320
path.parent.type === "VariableDeclaration" &&
296321
path.parentPath.parent.type !== "ForStatement" &&

src/convert/expressions.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,69 @@ describe("transform expressions", () => {
220220
});
221221
});
222222

223+
describe("React.AbstractComponent", () => {
224+
it("should handle anonymous component", async () => {
225+
const src = dedent`
226+
// @flow
227+
export default (memo<Props>((props) => null): AbstractComponent<Props, Ref>);
228+
`;
229+
const expected = dedent`
230+
export default memo<Props>((props) => null);
231+
`;
232+
expect(await transform(src)).toBe(expected);
233+
});
234+
235+
it("should handle named component", async () => {
236+
const src = dedent`
237+
// @flow
238+
export default (memo<Props>(Comp): AbstractComponent<Props, Ref>);
239+
`;
240+
const expected = dedent`
241+
export default memo<Props>(Comp);
242+
`;
243+
expect(await transform(src)).toBe(expected);
244+
});
245+
246+
it("should not remove when annotation is not for a function call", async () => {
247+
const src = dedent`
248+
export default function withFoo(f: Foo): <P>(S: AbstractComponent<P>) => AbstractComponent<P> {
249+
return function withFoo<P>(Subject: AbstractComponent<P>): AbstractComponent<P & InjectedP> {
250+
return () => null;
251+
};
252+
}
253+
`;
254+
expect(await transform(src)).toBe(src);
255+
});
256+
});
257+
258+
describe("React.forwardRef", () => {
259+
it("should handle anonymous component", async () => {
260+
const src = dedent`
261+
// @flow
262+
const C: AbstractComponent<Props, Ref> = forwardRef<Props, Ref>((props, ref) => null);
263+
export default (forwardRef<Props, Ref>((props, ref) => null): AbstractComponent<Props, Ref>);
264+
`;
265+
const expected = dedent`
266+
const C = forwardRef<Ref, Props>((props, ref) => null);
267+
export default forwardRef<Ref, Props>((props, ref) => null);
268+
`;
269+
expect(await transform(src)).toBe(expected);
270+
});
271+
272+
it("should handle named component", async () => {
273+
const src = dedent`
274+
// @flow
275+
const C: AbstractComponent<Props, mixed> = forwardRef<Props, Ref>(Comp);
276+
export default (forwardRef<Props, Ref>(Comp): AbstractComponent<Props, mixed>);
277+
`;
278+
const expected = dedent`
279+
const C = forwardRef<Ref, Props>(Comp);
280+
export default forwardRef<Ref, Props>(Comp);
281+
`;
282+
expect(await transform(src)).toBe(expected);
283+
});
284+
});
285+
223286
describe.each(JEST_MOCK_METHODS)("jest.%s paths", (mockMethod) => {
224287
it("should do nothing if there is no extension already", async () => {
225288
const src = dedent`jest.${mockMethod}('foo');`;

src/convert/expressions.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,15 @@ export function transformExpressions({
168168
);
169169
}
170170
},
171+
TSAsExpression(path) {
172+
if (
173+
path.node.typeAnnotation.type === "TSTypeReference" &&
174+
path.node.typeAnnotation.typeName.type === "Identifier" &&
175+
path.node.typeAnnotation.typeName.name === "AbstractComponent"
176+
) {
177+
path.replaceWith(path.node.expression);
178+
}
179+
},
171180
ArrowFunctionExpression(path) {
172181
// Arrow functions with a generic type parameter (<T>() => {}) often don't typecheck in tsx files
173182
// since they can be parsed as a JSX tag. To solve this the type parameters usually extend unknown,
@@ -188,6 +197,19 @@ export function transformExpressions({
188197
CallExpression(path) {
189198
migrateArgumentsToParameters(path, reporter, state);
190199

200+
// forwardRef<> in TS has the type parameters in reverse order
201+
// `forwardRef<Props, Ref>` → `forwardRef<Ref, Props>`
202+
if (
203+
path.node.type === "CallExpression" &&
204+
path.node.callee.type === "Identifier" &&
205+
path.node.callee.name === "forwardRef" &&
206+
path.node.typeParameters?.type === "TSTypeParameterInstantiation"
207+
) {
208+
path.node.typeParameters.params = path.node.typeParameters.params
209+
.concat()
210+
.reverse();
211+
}
212+
191213
if (
192214
t.isMemberExpression(path.node.callee) &&
193215
t.isIdentifier(path.node.callee.property) &&

0 commit comments

Comments
 (0)