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: Ensure added imports of types use the type modifier #343

Merged
merged 3 commits into from
Feb 15, 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
50 changes: 50 additions & 0 deletions .changeset/chilled-oranges-sit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
---
"types-react-codemod": patch
---

Ensure added imports of types use the `type` modifier

If we'd previously add an import to `ReactNode` (e.g. in `deprecated-react-fragment`),
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'
```

This also changes how we transform the deprecated global JSX namespace.
Instead of rewriting each usage, we opt for adding another import.
The guiding principle being that we keep the changes minimal in a codemod.

Before:

```diff
import * as React from 'react'

-const element: JSX.Element
+const element: React.JSX.Element
```

After:

```diff
import * as React from 'react'
+import { type JSX } from 'react'

const element: JSX.Element
```

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.
34 changes: 33 additions & 1 deletion transforms/__tests__/deprecated-react-fragment.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,23 @@ describe("transform deprecated-react-node-array", () => {
}
`),
).toMatchInlineSnapshot(`
"import { ReactNode } from 'react';
"import { type ReactNode } from 'react';
interface Props {
children?: Iterable<ReactNode>;
}"
`);
});

test("named type import", () => {
expect(
applyTransform(`
import type { ReactFragment } from 'react';
interface Props {
children?: ReactFragment;
}
`),
).toMatchInlineSnapshot(`
"import type { ReactNode } from 'react';
interface Props {
children?: Iterable<ReactNode>;
}"
Expand All @@ -63,6 +79,22 @@ describe("transform deprecated-react-node-array", () => {
`);
});

test("named import with existing ReactNode type import", () => {
expect(
applyTransform(`
import { ReactFragment, type ReactNode } from 'react';
interface Props {
children?: ReactFragment;
}
`),
).toMatchInlineSnapshot(`
"import { type ReactNode } from 'react';
interface Props {
children?: Iterable<ReactNode>;
}"
`);
});

test("false-negative named renamed import", () => {
expect(
applyTransform(`
Expand Down
34 changes: 33 additions & 1 deletion transforms/__tests__/deprecated-react-node-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,23 @@ describe("transform deprecated-react-node-array", () => {
}
`),
).toMatchInlineSnapshot(`
"import { ReactNode } from 'react';
"import { type ReactNode } from 'react';
interface Props {
children?: ReadonlyArray<ReactNode>;
}"
`);
});

test("named type import", () => {
expect(
applyTransform(`
import type { ReactNodeArray } from 'react';
interface Props {
children?: ReactNodeArray;
}
`),
).toMatchInlineSnapshot(`
"import type { ReactNode } from 'react';
interface Props {
children?: ReadonlyArray<ReactNode>;
}"
Expand All @@ -63,6 +79,22 @@ describe("transform deprecated-react-node-array", () => {
`);
});

test("named import with existing ReactNode type import", () => {
expect(
applyTransform(`
import { ReactNodeArray, type ReactNode } from 'react';
interface Props {
children?: ReactNodeArray;
}
`),
).toMatchInlineSnapshot(`
"import { type ReactNode } from 'react';
interface Props {
children?: ReadonlyArray<ReactNode>;
}"
`);
});

test("false-negative named renamed import", () => {
expect(
applyTransform(`
Expand Down
12 changes: 12 additions & 0 deletions transforms/__tests__/deprecated-sfc-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ describe("transform deprecated-sfc-element", () => {
`);
});

test("named typew import", () => {
expect(
applyTransform(`
import { type SFCElement } from 'react';
SFCElement;
`),
).toMatchInlineSnapshot(`
"import { type FunctionComponentElement } from 'react';
FunctionComponentElement;"
`);
});

test("named renamed import", () => {
expect(
applyTransform(`
Expand Down
12 changes: 12 additions & 0 deletions transforms/__tests__/deprecated-sfc.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ describe("transform deprecated-sfc", () => {
`);
});

test("named type import", () => {
expect(
applyTransform(`
import { type SFC } from 'react';
SFC;
`),
).toMatchInlineSnapshot(`
"import { type FC } from 'react';
FC;"
`);
});

test("named renamed import", () => {
expect(
applyTransform(`
Expand Down
12 changes: 12 additions & 0 deletions transforms/__tests__/deprecated-stateless-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ describe("transform deprecated-stateless-component", () => {
`);
});

test("named type import", () => {
expect(
applyTransform(`
import { type StatelessComponent } from 'react';
StatelessComponent;
`),
).toMatchInlineSnapshot(`
"import { type FunctionComponent } from 'react';
FunctionComponent;"
`);
});

test("named renamed import", () => {
expect(
applyTransform(`
Expand Down
14 changes: 14 additions & 0 deletions transforms/__tests__/deprecated-void-function-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,20 @@ describe("transform deprecated-void-function-component", () => {
`);
});

test("named type import", () => {
expect(
applyTransform(`
import { type VFC, type VoidFunctionComponent } from 'react';
VFC;
VoidFunctionComponent;
`),
).toMatchInlineSnapshot(`
"import { type FC, type FunctionComponent } from 'react';
FC;
FunctionComponent;"
`);
});

test("named renamed import", () => {
expect(
applyTransform(`
Expand Down
59 changes: 51 additions & 8 deletions transforms/__tests__/scoped-jsx.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,32 @@ describe("transform scoped-jsx", () => {
declare const element: JSX.Element;
`),
).toMatchInlineSnapshot(`
"import { JSX } from "react";
"import type { JSX } from "react";
declare const element: JSX.Element;"
`);
});

test("existing default import", () => {
expect(
applyTransform(`
import React from 'react';
declare const element: JSX.Element;
`),
).toMatchInlineSnapshot(`
"import React, { type JSX } from 'react';
declare const element: JSX.Element;"
`);
});

test("existing type default import", () => {
expect(
applyTransform(`
import type React from 'react';
declare const element: JSX.Element;
`),
).toMatchInlineSnapshot(`
"import type React from 'react';
import type { JSX } from "react";
declare const element: JSX.Element;"
`);
});
Expand All @@ -46,7 +71,8 @@ describe("transform scoped-jsx", () => {
`),
).toMatchInlineSnapshot(`
"import * as React from 'react';
declare const element: React.JSX.Element;"
import type { JSX } from "react";
declare const element: JSX.Element;"
`);
});

Expand All @@ -58,19 +84,34 @@ describe("transform scoped-jsx", () => {
`),
).toMatchInlineSnapshot(`
"import type * as React from 'react';
declare const element: React.JSX.Element;"
import type { JSX } from "react";
declare const element: JSX.Element;"
`);
});

test("existing named import", () => {
test("existing named import with other named imports", () => {
expect(
applyTransform(`
import { ReactNode } from 'react';
declare const element: JSX.Element;
declare const node: ReactNode;
`),
).toMatchInlineSnapshot(`
"import { ReactNode, JSX } from 'react';
"import { ReactNode, type JSX } from 'react';
declare const element: JSX.Element;
declare const node: ReactNode;"
`);
});

test("existing named import with other named type imports", () => {
expect(
applyTransform(`
import type { ReactNode } from 'react';
declare const element: JSX.Element;
declare const node: ReactNode;
`),
).toMatchInlineSnapshot(`
"import type { ReactNode, JSX } from 'react';
declare const element: JSX.Element;
declare const node: ReactNode;"
`);
Expand All @@ -95,7 +136,7 @@ describe("transform scoped-jsx", () => {
declare const element: JSX.Element;
`),
).toMatchInlineSnapshot(`
"import { JSX } from "react";
"import type { JSX } from "react";
const React = require('react');
declare const element: JSX.Element;"
`);
Expand All @@ -113,7 +154,7 @@ describe("transform scoped-jsx", () => {
"import {} from 'react-dom'
import {} from '@testing-library/react'

import { JSX } from "react";
import type { JSX } from "react";

declare const element: JSX.Element;"
`);
Expand All @@ -129,7 +170,9 @@ describe("transform scoped-jsx", () => {
).toMatchInlineSnapshot(`
"import * as React from 'react'

declare const attributes: React.JSX.LibraryManagedAttributes<A, B>;"
import type { JSX } from "react";

declare const attributes: JSX.LibraryManagedAttributes<A, B>;"
`);
});
});
12 changes: 9 additions & 3 deletions transforms/deprecated-react-fragment.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
const parseSync = require("./utils/parseSync");

/**
* @type {import('jscodeshift').Transform}
*/
Expand Down Expand Up @@ -29,8 +28,15 @@ const deprecatedReactFragmentTransform = (file, api) => {
if (hasReactNodeImport.length > 0) {
reactFragmentImports.remove();
} else {
reactFragmentImports.replaceWith(() => {
return j.importSpecifier(j.identifier("ReactNode"));
reactFragmentImports.replaceWith((path) => {
const importSpecifier = j.importSpecifier(j.identifier("ReactNode"));
const importDeclaration = path.parentPath.parentPath.value;
if (importDeclaration.importKind !== "type") {
// @ts-expect-error -- Missing types in jscodeshift. Babel uses `importKind`: https://astexplorer.net/#/gist/a76bd35f28483a467fef29d3c63aac9b/0e7ba6688fc09bd11b92197349b2384bb4c94574
importSpecifier.importKind = "type";
}

return importSpecifier;
});
}

Expand Down
12 changes: 10 additions & 2 deletions transforms/deprecated-react-node-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,16 @@ const deprecatedReactNodeArrayTransform = (file, api) => {
if (hasReactNodeImport.length > 0) {
reactNodeArrayImports.remove();
} else {
reactNodeArrayImports.replaceWith(() => {
return j.importSpecifier(j.identifier("ReactNode"));
reactNodeArrayImports.replaceWith((path) => {
const importSpecifier = j.importSpecifier(j.identifier("ReactNode"));

const importDeclaration = path.parentPath.parentPath.value;
if (importDeclaration.importKind !== "type") {
// @ts-expect-error -- Missing types in jscodeshift. Babel uses `importKind`: https://astexplorer.net/#/gist/a76bd35f28483a467fef29d3c63aac9b/0e7ba6688fc09bd11b92197349b2384bb4c94574
importSpecifier.importKind = "type";
}

return importSpecifier;
});
}

Expand Down
Loading