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

Add codemod for new RefObject behavior #208

Merged
merged 10 commits into from
Mar 29, 2023
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
7 changes: 7 additions & 0 deletions .changeset/fifty-pumpkins-work.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"types-react-codemod": minor
---

Add experimental (not for use in published types) codemod for new `RefObject` behavior

TODO link `@types/react` PR that landed this change
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,6 @@ dist
.yarn/build-state.yml
.yarn/install-state.gz
.pnp.*

# macOS
.DS_STORE
Binary file added .yarn/cache/fsevents-patch-2882183fbf-8.zip
Binary file not shown.
31 changes: 28 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ types-react-codemod <codemod> <paths...>
Positionals:
codemod [string] [required] [choices: "context-any", "deprecated-react-child",
"deprecated-react-text", "deprecated-react-type", "deprecated-sfc-element",
"deprecated-sfc", "deprecated-stateless-component", "implicit-children",
"preset-18", "preset-19", "useCallback-implicit-any"]
"deprecated-sfc", "deprecated-stateless-component",
"deprecated-void-function-component", "experimental-refobject-defaults",
"implicit-children", "preset-18", "preset-19", "useCallback-implicit-any"]
paths [string] [required]

Options:
Expand Down Expand Up @@ -215,7 +216,7 @@ You should select all and audit the changed files regardless.
}
```

#### `deprecated-react-text` false-negative pattern A
#### `deprecated-react-child` false-negative pattern A

Importing `ReactChild` via aliased named import will result in the transform being skipped.

Expand Down Expand Up @@ -262,6 +263,30 @@ In earlier versions of `@types/react` this codemod would change the typings.
+const Component: React.FunctionComponent = () => {}
```

### `experimental-refobject-defaults`

WARNING: This is an experimental codemod to intended for codebases using published types.
Only use if you're using https://github.com/DefinitelyTyped/DefinitelyTyped/pull/64896.

`RefObject` no longer makes `current` nullable by default

```diff
import * as React from "react";
-const myRef: React.RefObject<View>
+const myRef: React.RefObject<View | null>
```

#### `experimental-refobject-defaults` false-negative pattern A

Importing `RefObject` via aliased named import will result in the transform being skipped.

```tsx
import { RefObject as MyRefObject } from "react";

// not transformed
const myRef: MyRefObject<View>;
```

## Supported platforms

The following list contains officially supported runtimes.
Expand Down
4 changes: 2 additions & 2 deletions bin/__tests__/types-react-codemod.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ describe("types-react-codemod", () => {
codemod [string] [required] [choices: "context-any", "deprecated-react-child",
"deprecated-react-text", "deprecated-react-type", "deprecated-sfc-element",
"deprecated-sfc", "deprecated-stateless-component",
"deprecated-void-function-component", "implicit-children", "preset-18",
"preset-19", "useCallback-implicit-any"]
"deprecated-void-function-component", "experimental-refobject-defaults",
"implicit-children", "preset-18", "preset-19", "useCallback-implicit-any"]
paths [string] [required]

Options:
Expand Down
119 changes: 119 additions & 0 deletions transforms/__tests__/refobject-defaults.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
const { describe, expect, test } = require("@jest/globals");
const dedent = require("dedent");
const JscodeshiftTestUtils = require("jscodeshift/dist/testUtils");
const refobjectDefaultsTransform = require("../experimental-refobject-defaults");

function applyTransform(source, options = {}) {
return JscodeshiftTestUtils.applyTransform(
refobjectDefaultsTransform,
options,
{
path: "test.d.ts",
source: dedent(source),
}
);
}

describe("transform refobject-defaults", () => {
test("not modified", () => {
expect(
applyTransform(`
import * as React from 'react';
interface Props {
children?: ReactNode;
}
`)
).toMatchInlineSnapshot(`
"import * as React from 'react';
interface Props {
children?: ReactNode;
}"
`);
});

test("named import", () => {
expect(
applyTransform(`
import { RefObject } from 'react';
const myRef: RefObject<View> = createRef();
`)
).toMatchInlineSnapshot(`
"import { RefObject } from 'react';
const myRef: RefObject<View | null> = createRef();"
`);
});

test("false-negative named renamed import", () => {
expect(
applyTransform(`
import { RefObject as MyRefObject } from 'react';
const myRef: MyRefObject<View> = createRef();
`)
).toMatchInlineSnapshot(`
"import { RefObject as MyRefObject } from 'react';
const myRef: MyRefObject<View> = createRef();"
`);
});

test("namespace import", () => {
expect(
applyTransform(`
import * as React from 'react';
const myRef: React.RefObject<View> = createRef();
`)
).toMatchInlineSnapshot(`
"import * as React from 'react';
const myRef: React.RefObject<View | null> = createRef();"
`);
});

test("unions", () => {
expect(
applyTransform(`
import * as React from 'react';
const myRef: React.RefObject<number | string> = createRef();
`)
).toMatchInlineSnapshot(`
"import * as React from 'react';
const myRef: React.RefObject<number | string | null> = createRef();"
`);
});

test("no change on apparent nullable", () => {
expect(
applyTransform(`
import * as React from 'react';
const myRef: React.RefObject<null | number> = createRef();
`)
).toMatchInlineSnapshot(`
"import * as React from 'react';
const myRef: React.RefObject<null | number> = createRef();"
`);
});

test("no change on apparent any", () => {
expect(
applyTransform(`
import * as React from 'react';
const anyRef: React.RefObject<any> = createRef();
const stillAnyRef: React.RefObject<any | number> = createRef();
type AnyAlias = any;
const notApparentAny: React.RefObject<AnyAlias> = createRef();
`)
).toMatchInlineSnapshot(`
"import * as React from 'react';
const anyRef: React.RefObject<any> = createRef();
const stillAnyRef: React.RefObject<any | number> = createRef();
type AnyAlias = any;
const notApparentAny: React.RefObject<AnyAlias | null> = createRef();"
`);
});

test("regression test discovered in Klarna klapp repo", () => {
expect(
applyTransform(`new Map<string, React.RefObject<HTMLDivElement>>()`)
).toMatchInlineSnapshot(
`"new Map<string, React.RefObject<HTMLDivElement | null>>()"`
);
});
});
87 changes: 87 additions & 0 deletions transforms/experimental-refobject-defaults.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
const parseSync = require("./utils/parseSync");
const t = require("@babel/types");
const traverse = require("@babel/traverse").default;

/**
* @type {import('jscodeshift').Transform}
*
* Summary for Klarna's klapp@b560a65cd2
* 1 errors
* 37561 unmodified
* 1 skipped
* 82 ok
* Fixes all remaining issues outside of n_m
*/
const refObjectDefaultsTransform = (file) => {
const ast = parseSync(file);

let changedSome = false;

// ast.get("program").value is sufficient for unit tests but not actually running it on files
// TODO: How to test?
const traverseRoot = ast.paths()[0].value;
/**
* @type {import('@babel/types').TSTypeReference[]}
*/
const refObjectTypeReferences = [];
traverse(traverseRoot, {
TSTypeReference({ node: typeReference }) {
const { typeName } = typeReference;
const identifier =
typeName.type === "TSQualifiedName" ? typeName.right : typeName;

if (["RefObject"].includes(identifier.name)) {
refObjectTypeReferences.push(typeReference);
}
},
});

refObjectTypeReferences.forEach((typeReference) => {
const params = typeReference.typeParameters?.params;
if (params !== undefined) {
const [typeNode] = params;

/**
* @type {typeof typeNode | undefined}
*/
let nullableType;
if (typeNode.type === "TSUnionType") {
const typeIsApparentAny = typeNode.types.some((unionMember) => {
return unionMember.type === "TSAnyKeyword";
});
if (!typeIsApparentAny) {
const unionIsApparentlyNullable = typeNode.types.some(
(unionMember) => {
return unionMember.type === "TSNullKeyword";
}
);

nullableType = unionIsApparentlyNullable
? typeNode
: t.tsUnionType([...typeNode.types, t.tsNullKeyword()]);
}
} else {
if (typeNode.type !== "TSAnyKeyword") {
nullableType = t.tsUnionType([typeNode, t.tsNullKeyword()]);
}
}

if (nullableType !== undefined && nullableType !== typeNode) {
// Ideally we'd clone the `typeReference` path and add `typeParameters`.
// But I don't know if there's an API or better pattern for it.
typeReference.typeParameters = t.tsTypeParameterInstantiation([
nullableType,
]);
changedSome = true;
}
}
});

// Otherwise some files will be marked as "modified" because formatting changed
if (changedSome) {
return ast.toSource();
}
return file.source;
};

module.exports = refObjectDefaultsTransform;
4 changes: 4 additions & 0 deletions transforms/preset-19.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const deprecatedReactChildTransform = require("./deprecated-react-child");
const deprecatedReactTextTransform = require("./deprecated-react-text");
const deprecatedVoidFunctionComponentTransform = require("./deprecated-void-function-component");
const refobjectDefaultsTransform = require("./experimental-refobject-defaults");

/**
* @type {import('jscodeshift').Transform}
Expand All @@ -22,6 +23,9 @@ const transform = (file, api, options) => {
if (transformNames.has("deprecated-void-function-component")) {
transforms.push(deprecatedVoidFunctionComponentTransform);
}
if (transformNames.has("plain-refs")) {
transforms.push(refobjectDefaultsTransform);
}

let wasAlwaysSkipped = true;
const newSource = transforms.reduce((currentFileSource, transform) => {
Expand Down