Skip to content

Commit

Permalink
fix: Consistent behavior for rename and replace transforms
Browse files Browse the repository at this point in the history
  • Loading branch information
Sebastian Silbermann committed Feb 19, 2024
1 parent 230153b commit 91f56ce
Show file tree
Hide file tree
Showing 23 changed files with 474 additions and 613 deletions.
12 changes: 3 additions & 9 deletions .changeset/chilled-oranges-sit.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
21 changes: 21 additions & 0 deletions .changeset/famous-nails-destroy.md
Original file line number Diff line number Diff line change
@@ -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`
2 changes: 0 additions & 2 deletions .changeset/short-zoos-type.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
28 changes: 14 additions & 14 deletions transforms/__tests__/deprecated-react-child.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ function applyTransform(source, options = {}) {
{
path: "test.d.ts",
source: dedent(source),
}
},
);
}

Expand All @@ -21,7 +21,7 @@ test("not modified", () => {
interface Props {
children?: ReactNode;
}
`)
`),
).toMatchInlineSnapshot(`
"import * as React from 'react';
interface Props {
Expand All @@ -37,11 +37,11 @@ test("named import", () => {
interface Props {
children?: ReactChild;
}
`)
`),
).toMatchInlineSnapshot(`
"import { ReactChild } from 'react';
"import { ReactElement } from 'react';
interface Props {
children?: React.ReactElement | number | string;
children?: ReactElement | number | string;
}"
`);
});
Expand All @@ -53,11 +53,11 @@ test("named type import", () => {
interface Props {
children?: ReactChild;
}
`)
`),
).toMatchInlineSnapshot(`
"import { type ReactChild } from 'react';
"import { type ReactElement } from 'react';
interface Props {
children?: React.ReactElement | number | string;
children?: ReactElement | number | string;
}"
`);
});
Expand All @@ -69,11 +69,11 @@ test("named type import with existing target import", () => {
interface Props {
children?: ReactChild;
}
`)
`),
).toMatchInlineSnapshot(`
"import { ReactChild, ReactElement } from 'react';
"import { ReactElement } from 'react';
interface Props {
children?: React.ReactElement | number | string;
children?: ReactElement | number | string;
}"
`);
});
Expand All @@ -85,7 +85,7 @@ test("false-negative named renamed import", () => {
interface Props {
children?: MyReactChild;
}
`)
`),
).toMatchInlineSnapshot(`
"import { ReactChild as MyReactChild } from 'react';
interface Props {
Expand All @@ -101,7 +101,7 @@ test("namespace import", () => {
interface Props {
children?: React.ReactChild;
}
`)
`),
).toMatchInlineSnapshot(`
"import * as React from 'react';
interface Props {
Expand All @@ -115,7 +115,7 @@ test("as type parameter", () => {
applyTransform(`
import * as React from 'react';
createAction<React.ReactChild>()
`)
`),
).toMatchInlineSnapshot(`
"import * as React from 'react';
createAction<React.ReactElement | number | string>()"
Expand Down
2 changes: 1 addition & 1 deletion transforms/__tests__/deprecated-react-fragment.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ test("named import", () => {
}
`),
).toMatchInlineSnapshot(`
"import { type ReactNode } from 'react';
"import { ReactNode } from 'react';
interface Props {
children?: Iterable<ReactNode>;
}"
Expand Down
2 changes: 1 addition & 1 deletion transforms/__tests__/deprecated-react-node-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ test("named import", () => {
}
`),
).toMatchInlineSnapshot(`
"import { type ReactNode } from 'react';
"import { ReactNode } from 'react';
interface Props {
children?: ReadonlyArray<ReactNode>;
}"
Expand Down
4 changes: 2 additions & 2 deletions transforms/__tests__/deprecated-react-text.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ test("named import", () => {
}
`),
).toMatchInlineSnapshot(`
"import { ReactText } from 'react';
"import 'react';
interface Props {
children?: number | string;
}"
Expand All @@ -55,7 +55,7 @@ test("named type import", () => {
}
`),
).toMatchInlineSnapshot(`
"import { type ReactText } from 'react';
"import 'react';
interface Props {
children?: number | string;
}"
Expand Down
82 changes: 41 additions & 41 deletions transforms/__tests__/deprecated-react-type.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,101 +10,101 @@ function applyTransform(source, options = {}) {
{
path: "test.d.ts",
source: dedent(source),
}
},
);
}

test("not modified", () => {
expect(
applyTransform(`
import { ElementType } from 'react';
ElementType;
`)
declare const a: ElementType;
`),
).toMatchInlineSnapshot(`
"import { ElementType } from 'react';
ElementType;"
declare const a: ElementType;"
`);
});

test("named import", () => {
expect(
applyTransform(`
import { ReactType } from 'react';
ReactType;
ReactType<T>;
`)
import { ReactType } from 'react';
declare const a: ReactType;
declare const b: ReactType<T>;
`),
).toMatchInlineSnapshot(`
"import { ElementType } from 'react';
ElementType;
ElementType<T>;"
declare const a: ElementType;
declare const b: ElementType<T>;"
`);
});

test("named type import", () => {
expect(
applyTransform(`
import { type ReactType } from 'react';
ReactType;
ReactType<T>;
`)
import { type ReactType } from 'react';
declare const a: ReactType;
declare const b: ReactType<T>;
`),
).toMatchInlineSnapshot(`
"import { type ElementType } from 'react';
ElementType;
ElementType<T>;"
declare const a: ElementType;
declare const b: ElementType<T>;"
`);
});

test("named type import with existing target import", () => {
expect(
applyTransform(`
import { type ReactType, ElementType } from 'react';
ReactType;
ReactType<T>;
`)
import { type ReactType, ElementType } from 'react';
declare const a: ReactType;
declare const b: ReactType<T>;
`),
).toMatchInlineSnapshot(`
"import { type ElementType, ElementType } from 'react';
ElementType;
ElementType<T>;"
"import { ElementType } from 'react';
declare const a: ElementType;
declare const b: ElementType<T>;"
`);
});

test("false-negative named renamed import", () => {
expect(
applyTransform(`
import { ReactType as MyReactType } from 'react';
MyReactType;
MyReactType<T>;
`)
import { ReactType as MyReactType } from 'react';
declare const a: MyReactType;
declare const b: MyReactType<T>;
`),
).toMatchInlineSnapshot(`
"import { ElementType as MyReactType } from 'react';
MyReactType;
MyReactType<T>;"
"import { ReactType as MyReactType } from 'react';
declare const a: MyReactType;
declare const b: MyReactType<T>;"
`);
});

test("namespace import", () => {
expect(
applyTransform(`
import * as React from 'react';
React.ReactType;
React.ReactType<T>;
`)
declare const a: React.ReactType;
declare const b: React.ReactType<T>;
`),
).toMatchInlineSnapshot(`
"import * as React from 'react';
React.ElementType;
React.ElementType<T>;"
declare const a: React.ElementType;
declare const b: React.ElementType<T>;"
`);
});

test("false-positive rename on different namespace", () => {
expect(
applyTransform(`
import * as Preact from 'preact';
Preact.ReactType;
`)
declare const a: Preact.ReactType;
`),
).toMatchInlineSnapshot(`
"import * as Preact from 'preact';
Preact.ElementType;"
declare const a: Preact.ElementType;"
`);
});

Expand All @@ -114,10 +114,10 @@ test("as type parameter", () => {
import * as React from 'react';
createComponent<React.ReactType>();
createComponent<React.ReactType<T>>();
`)
`),
).toMatchInlineSnapshot(`
"import * as React from 'react';
createComponent<React.ReactType>();
createComponent<React.ReactType<T>>();"
createComponent<React.ElementType>();
createComponent<React.ElementType<T>>();"
`);
});
Loading

0 comments on commit 91f56ce

Please sign in to comment.