Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Consistent behavior for rename and replace transforms #348

Merged
merged 2 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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`
88 changes: 59 additions & 29 deletions transforms/__tests__/deprecated-react-child.js
Original file line number Diff line number Diff line change
@@ -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");
Expand All @@ -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>()"
`);
});
});
88 changes: 43 additions & 45 deletions transforms/__tests__/deprecated-react-fragment.js
Original file line number Diff line number Diff line change
@@ -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");
Expand All @@ -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>>();"
`);
});
});
Loading