From 165aad5884d357e4eaf325e721102be927870a6f Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Thu, 15 Feb 2024 09:13:32 +0100 Subject: [PATCH] fix: Find and replace type usage in type parameters of call expressions --- .changeset/short-zoos-type.md | 15 ++++++ .../__tests__/deprecated-react-child.js | 12 +++++ .../__tests__/deprecated-react-fragment.js | 12 +++++ .../__tests__/deprecated-react-node-array.js | 12 +++++ transforms/__tests__/deprecated-react-text.js | 12 +++++ transforms/__tests__/scoped-jsx.js | 18 ++++++- transforms/deprecated-react-child.js | 23 +++++++-- transforms/deprecated-react-fragment.js | 46 +++++++++++++----- transforms/deprecated-react-node-array.js | 48 ++++++++++++++----- transforms/deprecated-react-text.js | 24 ++++++++-- transforms/scoped-jsx.js | 32 ++++++++----- transforms/utils/jscodeshift-bugfixes.js | 23 +++++++++ 12 files changed, 228 insertions(+), 49 deletions(-) create mode 100644 .changeset/short-zoos-type.md create mode 100644 transforms/utils/jscodeshift-bugfixes.js diff --git a/.changeset/short-zoos-type.md b/.changeset/short-zoos-type.md new file mode 100644 index 00000000..00850b3a --- /dev/null +++ b/.changeset/short-zoos-type.md @@ -0,0 +1,15 @@ +--- +"types-react-codemod": patch +--- + +Find and replace type usage in type parameters of call expressions + +Now we properly detect that e.g. `JSX` is used in `someFunctionWithTypeParameters()`. + +Affected codemods: + +- `deprecated-react-child` +- `deprecated-react-fragment` +- `deprecated-react-node-array` +- `deprecated-react-text` +- `scoped-jsx` diff --git a/transforms/__tests__/deprecated-react-child.js b/transforms/__tests__/deprecated-react-child.js index 79044ca9..0e72b70e 100644 --- a/transforms/__tests__/deprecated-react-child.js +++ b/transforms/__tests__/deprecated-react-child.js @@ -78,4 +78,16 @@ describe("transform deprecated-react-child", () => { }" `); }); + + test("as type parameter", () => { + expect( + applyTransform(` + import * as React from 'react'; + createAction() + `), + ).toMatchInlineSnapshot(` + "import * as React from 'react'; + createAction()" + `); + }); }); diff --git a/transforms/__tests__/deprecated-react-fragment.js b/transforms/__tests__/deprecated-react-fragment.js index 9f0c0489..ff474eee 100644 --- a/transforms/__tests__/deprecated-react-fragment.js +++ b/transforms/__tests__/deprecated-react-fragment.js @@ -126,4 +126,16 @@ describe("transform deprecated-react-node-array", () => { }" `); }); + + test("as type parameter", () => { + expect( + applyTransform(` + import * as React from 'react'; + createComponent(); + `), + ).toMatchInlineSnapshot(` + "import * as React from 'react'; + createComponent>();" + `); + }); }); diff --git a/transforms/__tests__/deprecated-react-node-array.js b/transforms/__tests__/deprecated-react-node-array.js index 21fa8fd9..b9e32047 100644 --- a/transforms/__tests__/deprecated-react-node-array.js +++ b/transforms/__tests__/deprecated-react-node-array.js @@ -126,4 +126,16 @@ describe("transform deprecated-react-node-array", () => { }" `); }); + + test("in type parameters", () => { + expect( + applyTransform(` + import * as React from 'react'; + createComponent(); + `), + ).toMatchInlineSnapshot(` + "import * as React from 'react'; + createComponent>();" + `); + }); }); diff --git a/transforms/__tests__/deprecated-react-text.js b/transforms/__tests__/deprecated-react-text.js index 182f753b..3b2d29f6 100644 --- a/transforms/__tests__/deprecated-react-text.js +++ b/transforms/__tests__/deprecated-react-text.js @@ -78,4 +78,16 @@ describe("transform deprecated-react-text", () => { }" `); }); + + test("in type parameters", () => { + expect( + applyTransform(` + import * as React from 'react'; + createComponent(); + `), + ).toMatchInlineSnapshot(` + "import * as React from 'react'; + createComponent();" + `); + }); }); diff --git a/transforms/__tests__/scoped-jsx.js b/transforms/__tests__/scoped-jsx.js index c0d1837f..274ec982 100644 --- a/transforms/__tests__/scoped-jsx.js +++ b/transforms/__tests__/scoped-jsx.js @@ -5,7 +5,7 @@ const scopedJSXTransform = require("../scoped-jsx"); function applyTransform(source, options = {}) { return JscodeshiftTestUtils.applyTransform(scopedJSXTransform, options, { - path: "test.d.ts", + path: "test.tsx", source: dedent(source), }); } @@ -175,4 +175,20 @@ describe("transform scoped-jsx", () => { declare const attributes: JSX.LibraryManagedAttributes;" `); }); + + test("detects usage in type parameters", () => { + expect( + applyTransform(` + import React, { useMemo } from 'react' + + [].reduce((acc, component, i) => {}); + [].reduce((acc, component, i) => {}) + `), + ).toMatchInlineSnapshot(` + "import React, { useMemo, type JSX } from 'react'; + + [].reduce((acc, component, i) => {}); + [].reduce((acc, component, i) => {})" + `); + }); }); diff --git a/transforms/deprecated-react-child.js b/transforms/deprecated-react-child.js index dc3a2aa9..dc4ab075 100644 --- a/transforms/deprecated-react-child.js +++ b/transforms/deprecated-react-child.js @@ -1,4 +1,7 @@ const parseSync = require("./utils/parseSync"); +const { + findTSTypeReferenceCollections, +} = require("./utils/jscodeshift-bugfixes"); /** * @type {import('jscodeshift').Transform} @@ -7,8 +10,10 @@ const deprecatedReactChildTransform = (file, api) => { const j = api.jscodeshift; const ast = parseSync(file); - const changedIdentifiers = ast - .find(j.TSTypeReference, (node) => { + const reactChildTypeReferences = findTSTypeReferenceCollections( + j, + ast, + (node) => { const { typeName } = node; /** * @type {import('jscodeshift').Identifier | null} @@ -24,8 +29,12 @@ const deprecatedReactChildTransform = (file, api) => { } return identifier !== null && identifier.name === "ReactChild"; - }) - .replaceWith(() => { + }, + ); + + let didChangeIdentifiers = false; + for (const typeReferences of reactChildTypeReferences) { + const changedIdentifiers = typeReferences.replaceWith(() => { // `React.ReactElement | number | string` return j.tsUnionType([ // React.ReactElement @@ -39,9 +48,13 @@ const deprecatedReactChildTransform = (file, api) => { j.tsStringKeyword(), ]); }); + if (changedIdentifiers.length > 0) { + didChangeIdentifiers = true; + } + } // Otherwise some files will be marked as "modified" because formatting changed - if (changedIdentifiers.length > 0) { + if (didChangeIdentifiers > 0) { return ast.toSource(); } return file.source; diff --git a/transforms/deprecated-react-fragment.js b/transforms/deprecated-react-fragment.js index 9da7a28f..d4548127 100644 --- a/transforms/deprecated-react-fragment.js +++ b/transforms/deprecated-react-fragment.js @@ -1,4 +1,7 @@ const parseSync = require("./utils/parseSync"); +const { + findTSTypeReferenceCollections, +} = require("./utils/jscodeshift-bugfixes"); /** * @type {import('jscodeshift').Transform} */ @@ -6,6 +9,8 @@ const deprecatedReactFragmentTransform = (file, api) => { const j = api.jscodeshift; const ast = parseSync(file); + let hasChanges = false; + const hasReactNodeImport = ast.find(j.ImportSpecifier, (node) => { const { imported, local } = node; return ( @@ -24,6 +29,9 @@ const deprecatedReactFragmentTransform = (file, api) => { (local == null || local.name === "ReactFragment") ); }); + if (reactFragmentImports.length > 0) { + hasChanges = true; + } if (hasReactNodeImport.length > 0) { reactFragmentImports.remove(); @@ -40,15 +48,19 @@ const deprecatedReactFragmentTransform = (file, api) => { }); } - const changedIdentifiers = ast - .find(j.TSTypeReference, (node) => { + const reactFragmentTypeReferences = findTSTypeReferenceCollections( + j, + ast, + (node) => { const { typeName } = node; return ( typeName.type === "Identifier" && typeName.name === "ReactFragment" ); - }) - .replaceWith(() => { + }, + ); + for (const typeReferences of reactFragmentTypeReferences) { + const changedIdentifiers = typeReferences.replaceWith(() => { // `Iterable` return j.tsTypeReference( j.identifier("Iterable"), @@ -57,9 +69,15 @@ const deprecatedReactFragmentTransform = (file, api) => { ]), ); }); + if (changedIdentifiers.length > 0) { + hasChanges = true; + } + } - const changedQualifiedNames = ast - .find(j.TSTypeReference, (node) => { + const reactFragmentQualifiedNamesReferences = findTSTypeReferenceCollections( + j, + ast, + (node) => { const { typeName } = node; return ( @@ -67,8 +85,10 @@ const deprecatedReactFragmentTransform = (file, api) => { typeName.right.type === "Identifier" && typeName.right.name === "ReactFragment" ); - }) - .replaceWith((path) => { + }, + ); + for (const typeReferences of reactFragmentQualifiedNamesReferences) { + const changedQualifiedNames = typeReferences.replaceWith((path) => { const { node } = path; const typeName = /** @type {import('jscodeshift').TSQualifiedName} */ ( node.typeName @@ -83,13 +103,13 @@ const deprecatedReactFragmentTransform = (file, api) => { ]), ); }); + if (changedQualifiedNames.length > 0) { + hasChanges = true; + } + } // Otherwise some files will be marked as "modified" because formatting changed - if ( - changedIdentifiers.length > 0 || - changedQualifiedNames.length > 0 || - reactFragmentImports.length > 0 - ) { + if (hasChanges) { return ast.toSource(); } return file.source; diff --git a/transforms/deprecated-react-node-array.js b/transforms/deprecated-react-node-array.js index ed6cbbc6..3d451468 100644 --- a/transforms/deprecated-react-node-array.js +++ b/transforms/deprecated-react-node-array.js @@ -1,4 +1,7 @@ const parseSync = require("./utils/parseSync"); +const { + findTSTypeReferenceCollections, +} = require("./utils/jscodeshift-bugfixes"); /** * @type {import('jscodeshift').Transform} @@ -7,6 +10,8 @@ const deprecatedReactNodeArrayTransform = (file, api) => { const j = api.jscodeshift; const ast = parseSync(file); + let hasChanges = false; + const hasReactNodeImport = ast.find(j.ImportSpecifier, (node) => { const { imported, local } = node; return ( @@ -25,6 +30,9 @@ const deprecatedReactNodeArrayTransform = (file, api) => { (local == null || local.name === "ReactNodeArray") ); }); + if (reactNodeArrayImports.length > 0) { + hasChanges = true; + } if (hasReactNodeImport.length > 0) { reactNodeArrayImports.remove(); @@ -42,15 +50,19 @@ const deprecatedReactNodeArrayTransform = (file, api) => { }); } - const changedIdentifiers = ast - .find(j.TSTypeReference, (node) => { + const reactNodeArrayTypeReferences = findTSTypeReferenceCollections( + j, + ast, + (node) => { const { typeName } = node; return ( typeName.type === "Identifier" && typeName.name === "ReactNodeArray" ); - }) - .replaceWith(() => { + }, + ); + for (const typeReferences of reactNodeArrayTypeReferences) { + const changedIdentifiers = typeReferences.replaceWith(() => { // `ReadonlyArray` return j.tsTypeReference( j.identifier("ReadonlyArray"), @@ -60,8 +72,15 @@ const deprecatedReactNodeArrayTransform = (file, api) => { ); }); - const changedQualifiedNames = ast - .find(j.TSTypeReference, (node) => { + if (changedIdentifiers.length > 0) { + hasChanges = true; + } + } + + const reactNodeArrayQualifiedTypeReferences = findTSTypeReferenceCollections( + j, + ast, + (node) => { const { typeName } = node; return ( @@ -69,8 +88,10 @@ const deprecatedReactNodeArrayTransform = (file, api) => { typeName.right.type === "Identifier" && typeName.right.name === "ReactNodeArray" ); - }) - .replaceWith((path) => { + }, + ); + for (const typeReferences of reactNodeArrayQualifiedTypeReferences) { + const changedQualifiedNames = typeReferences.replaceWith((path) => { const { node } = path; const typeName = /** @type {import('jscodeshift').TSQualifiedName} */ ( node.typeName @@ -86,12 +107,13 @@ const deprecatedReactNodeArrayTransform = (file, api) => { ); }); + if (changedQualifiedNames.length > 0) { + hasChanges = true; + } + } + // Otherwise some files will be marked as "modified" because formatting changed - if ( - changedIdentifiers.length > 0 || - changedQualifiedNames.length > 0 || - reactNodeArrayImports.length > 0 - ) { + if (hasChanges) { return ast.toSource(); } return file.source; diff --git a/transforms/deprecated-react-text.js b/transforms/deprecated-react-text.js index 57dae31b..ebecec17 100644 --- a/transforms/deprecated-react-text.js +++ b/transforms/deprecated-react-text.js @@ -1,4 +1,7 @@ const parseSync = require("./utils/parseSync"); +const { + findTSTypeReferenceCollections, +} = require("./utils/jscodeshift-bugfixes"); /** * @type {import('jscodeshift').Transform} @@ -7,8 +10,12 @@ const deprecatedReactTextTransform = (file, api) => { const j = api.jscodeshift; const ast = parseSync(file); - const changedIdentifiers = ast - .find(j.TSTypeReference, (node) => { + let hasChanges = false; + + const reactTextTypeReferences = findTSTypeReferenceCollections( + j, + ast, + (node) => { const { typeName } = node; /** * @type {import('jscodeshift').Identifier | null} @@ -24,14 +31,21 @@ const deprecatedReactTextTransform = (file, api) => { } return identifier !== null && identifier.name === "ReactText"; - }) - .replaceWith(() => { + }, + ); + + for (const typeReferences of reactTextTypeReferences) { + const changedIdentifiers = typeReferences.replaceWith(() => { // `number | string` return j.tsUnionType([j.tsNumberKeyword(), j.tsStringKeyword()]); }); + if (changedIdentifiers.length > 0) { + hasChanges = true; + } + } // Otherwise some files will be marked as "modified" because formatting changed - if (changedIdentifiers.length > 0) { + if (hasChanges) { return ast.toSource(); } return file.source; diff --git a/transforms/scoped-jsx.js b/transforms/scoped-jsx.js index f61f917b..b2482932 100644 --- a/transforms/scoped-jsx.js +++ b/transforms/scoped-jsx.js @@ -3,6 +3,7 @@ const { addReactTypeImport, findReactImportForNewImports, } = require("./utils/newReactImports"); +const traverse = require("@babel/traverse").default; /** * @type {import('jscodeshift').Transform} @@ -11,21 +12,28 @@ const deprecatedReactChildTransform = (file, api) => { const j = api.jscodeshift; const ast = parseSync(file); - const globalNamespaceReferences = ast.find(j.TSTypeReference, (node) => { - const { typeName } = node; + let hasChanges = false; + let hasGlobalNamespaceReferences = false; - if (typeName.type === "TSQualifiedName") { - return ( - typeName.left.type === "Identifier" && - typeName.left.name === "JSX" && - typeName.right.type === "Identifier" - ); - } - return 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; + traverse(traverseRoot, { + TSTypeReference({ node: typeReference }) { + const { typeName } = typeReference; + if (typeName.type === "TSQualifiedName") { + if ( + typeName.left.type === "Identifier" && + typeName.left.name === "JSX" && + typeName.right.type === "Identifier" + ) { + hasGlobalNamespaceReferences = true; + } + } + }, }); - let hasChanges = false; - if (globalNamespaceReferences.length > 0) { + if (hasGlobalNamespaceReferences) { const reactImport = findReactImportForNewImports(j, ast); const jsxImportSpecifier = reactImport.find(j.ImportSpecifier, { imported: { name: "JSX" }, diff --git a/transforms/utils/jscodeshift-bugfixes.js b/transforms/utils/jscodeshift-bugfixes.js new file mode 100644 index 00000000..46c62734 --- /dev/null +++ b/transforms/utils/jscodeshift-bugfixes.js @@ -0,0 +1,23 @@ +/** + * This function returns all TSTypeReference collections in the AST. + * + * ast.find(TSTypeReference) does not find type references in type parameters of call expression. + * @param {import('jscodeshift').API['jscodeshift']} j + * @param {import('jscodeshift').Collection} ast + * @param {(path: import('jscodeshift').TSTypeReference) => boolean} filter + */ +function findTSTypeReferenceCollections(j, ast, filter) { + return [ + ast.find(j.TSTypeReference, filter), + ast + .find(j.CallExpression, (node) => { + return node.typeParameters !== undefined; + }) + .map((path) => { + return path.get("typeParameters"); + }) + .find(j.TSTypeReference, filter), + ]; +} + +module.exports = { findTSTypeReferenceCollections };