feat: Add no-implicit-ref-callback-return transform (#369)

Author: eps1lon

.changeset/thick-snakes-float.md

@@ -0,0 +1,15 @@
+---
+"types-react-codemod": minor
+---
+
+Add `no-implicit-ref-callback-return` transform
+
+Ensures you don't accidentally return anything from ref callbacks since the return value was always ignored.
+With ref cleanups, this is no longer the case and flagged in types to avoid mistakes.
+
+```diff
+-<div ref={current => (instance = current)} />
++<div ref={current => {instance = current}} />
+```
+
+The transform is opt-in in the `preset-19` in case you already used ref cleanups in Canary releases.

README.md

@@ -38,8 +38,9 @@ Positionals:
                      "deprecated-react-fragment", "deprecated-react-node-array",
      "deprecated-react-text", "deprecated-react-type", "deprecated-sfc-element",
                              "deprecated-sfc", "deprecated-stateless-component",
-         "deprecated-void-function-component", "implicit-children", "preset-18",
-    "preset-19", "refobject-defaults", "scoped-jsx", "useCallback-implicit-any",
+                      "deprecated-void-function-component", "implicit-children",
+                    "no-implicit-ref-callback-return", "preset-18", "preset-19",
+                 "refobject-defaults", "scoped-jsx", "useCallback-implicit-any",
                                                       "useRef-required-initial"]
   paths                                                      [string] [required]
 
@@ -353,6 +354,23 @@ In earlier versions of `@types/react` this codemod would change the typings.
 +const Component: React.FunctionComponent = () => {}
 ```
 
+### `no-implicit-ref-callback-return`
+
+Off by default in `preset-19`. Can be enabled when running `preset-19`.
+
+WARNING: Manually review changes in case you already used ref cleanups in Canary builds.
+
+Ensures you don't accidentally return anything from ref callbacks since the return value was always ignored.
+With ref cleanups, this is no longer the case and flagged in types to avoid mistakes.
+
+```diff
+-<div ref={current => (instance = current)} />
++<div ref={current => {instance = current}} />
+```
+
+This only works for the `ref` prop.
+The codemod will not apply to other props that take refs (e.g. `innerRef`).
+
 ### `refobject-defaults`
 
 WARNING: This is an experimental codemod to intended for codebases using unpublished types.

bin/__tests__/types-react-codemod.js

@@ -26,8 +26,9 @@ describe("types-react-codemod", () => {
 		                     "deprecated-react-fragment", "deprecated-react-node-array",
 		     "deprecated-react-text", "deprecated-react-type", "deprecated-sfc-element",
 		                             "deprecated-sfc", "deprecated-stateless-component",
-		         "deprecated-void-function-component", "implicit-children", "preset-18",
-		    "preset-19", "refobject-defaults", "scoped-jsx", "useCallback-implicit-any",
+		                      "deprecated-void-function-component", "implicit-children",
+		                    "no-implicit-ref-callback-return", "preset-18", "preset-19",
+		                 "refobject-defaults", "scoped-jsx", "useCallback-implicit-any",
 		                                                      "useRef-required-initial"]
 		  paths                                                      [string] [required]
 

bin/types-react-codemod.cjs

@@ -105,6 +105,7 @@ async function main() {
 								{ checked: true, value: "deprecated-react-fragment" },
 								{ checked: true, value: "deprecated-react-text" },
 								{ checked: true, value: "deprecated-void-function-component" },
+								{ checked: false, value: "no-implicit-ref-callback-return" },
 								{ checked: true, value: "refobject-defaults" },
 								{ checked: true, value: "scoped-jsx" },
 								{ checked: true, value: "useRef-required-initial" },

transforms/__tests__/no-implicit-ref-callback-return.js

@@ -0,0 +1,67 @@
+const { expect, test } = require("@jest/globals");
+const dedent = require("dedent");
+const JscodeshiftTestUtils = require("jscodeshift/dist/testUtils");
+const noImplicitRefCallbackReturnTransform = require("../no-implicit-ref-callback-return");
+
+function applyTransform(source, options = {}) {
+	return JscodeshiftTestUtils.applyTransform(
+		noImplicitRefCallbackReturnTransform,
+		options,
+		{
+			path: "test.tsx",
+			source: dedent(source),
+		},
+	);
+}
+
+test("not modified", () => {
+	expect(
+		applyTransform(`
+				<div ref={current => {instance = current}} />
+    `),
+	).toMatchInlineSnapshot(`"<div ref={current => {instance = current}} />"`);
+});
+
+test("replaces implicit return of assignment expression with block", () => {
+	expect(
+		applyTransform(`
+				<div ref={current => (instance = current)} />
+    `),
+	).toMatchInlineSnapshot(`
+		"<div ref={current => {
+		  (instance = current);
+		}} />"
+	`);
+});
+
+test("replaces implicit return of identifier with block", () => {
+	expect(
+		applyTransform(`
+				<div ref={current => current} />
+    `),
+	).toMatchInlineSnapshot(`
+		"<div ref={current => {
+		  current;
+		}} />"
+	`);
+});
+
+test("function expression", () => {
+	expect(
+		applyTransform(`
+				<div ref={function (current) { instance = current }} />
+    `),
+	).toMatchInlineSnapshot(
+		`"<div ref={function (current) { instance = current }} />"`,
+	);
+});
+
+test("only applies to `ref` prop", () => {
+	expect(
+		applyTransform(`
+				<div innerRef={current => (instance = current)} />
+    `),
+	).toMatchInlineSnapshot(
+		`"<div innerRef={current => (instance = current)} />"`,
+	);
+});

transforms/__tests__/preset-19.js

@@ -11,6 +11,7 @@ describe("preset-19", () => {
 	let deprecatedReactFragmentTransform;
 	let deprecatedReactTextTransform;
 	let deprecatedVoidFunctionComponentTransform;
+	let noImplicitRefCallbackReturnTransform;
 	let refobjectDefaultsTransform;
 	let scopedJSXTransform;
 	let useRefRequiredInitialTransform;
@@ -48,6 +49,9 @@ describe("preset-19", () => {
 		deprecatedVoidFunctionComponentTransform = mockTransform(
 			"../deprecated-void-function-component",
 		);
+		noImplicitRefCallbackReturnTransform = mockTransform(
+			"../no-implicit-ref-callback-return",
+		);
 		refobjectDefaultsTransform = mockTransform("../refobject-defaults");
 		scopedJSXTransform = mockTransform("../scoped-jsx");
 		useRefRequiredInitialTransform = mockTransform(
@@ -77,6 +81,7 @@ describe("preset-19", () => {
 				"deprecated-react-node-array",
 				"deprecated-react-text",
 				"deprecated-void-function-component",
+				"no-implicit-ref-callback-return",
 				"refobject-defaults",
 				"scoped-jsx",
 				"useRef-required-initial",
@@ -90,6 +95,7 @@ describe("preset-19", () => {
 		expect(deprecatedReactFragmentTransform).toHaveBeenCalled();
 		expect(deprecatedReactTextTransform).toHaveBeenCalled();
 		expect(deprecatedVoidFunctionComponentTransform).toHaveBeenCalled();
+		expect(noImplicitRefCallbackReturnTransform).toHaveBeenCalled();
 		expect(refobjectDefaultsTransform).toHaveBeenCalled();
 		expect(scopedJSXTransform).toHaveBeenCalled();
 		expect(useRefRequiredInitialTransform).toHaveBeenCalled();

transforms/no-implicit-ref-callback-return.js

@@ -0,0 +1,38 @@
+const parseSync = require("./utils/parseSync");
+
+/**
+ * @type {import('jscodeshift').Transform}
+ */
+const noImplicitRefCallbackReturnTransform = (file, api) => {
+	const j = api.jscodeshift;
+	const ast = parseSync(file);
+
+	let changedSome = false;
+
+	ast
+		.find(j.JSXAttribute, (jsxAttribute) => {
+			return jsxAttribute.name.name === "ref";
+		})
+		.forEach((jsxAttributePath) => {
+			const jsxAttribute = jsxAttributePath.node;
+			if (
+				jsxAttribute.value?.type === "JSXExpressionContainer" &&
+				jsxAttribute.value.expression.type === "ArrowFunctionExpression" &&
+				jsxAttribute.value.expression.body.type !== "BlockStatement"
+			) {
+				changedSome = true;
+
+				jsxAttribute.value.expression.body = j.blockStatement([
+					j.expressionStatement(jsxAttribute.value.expression.body),
+				]);
+			}
+		});
+
+	// Otherwise some files will be marked as "modified" because formatting changed
+	if (changedSome) {
+		return ast.toSource();
+	}
+	return file.source;
+};
+
+module.exports = noImplicitRefCallbackReturnTransform;

transforms/preset-19.js

@@ -5,6 +5,7 @@ const deprecatedReactNodeArrayTransform = require("./deprecated-react-node-array
 const deprecatedReactFragmentTransform = require("./deprecated-react-fragment");
 const deprecatedReactTextTransform = require("./deprecated-react-text");
 const deprecatedVoidFunctionComponentTransform = require("./deprecated-void-function-component");
+const noImplicitRefCallbackReturnTransform = require("./no-implicit-ref-callback-return");
 const refobjectDefaultsTransform = require("./refobject-defaults");
 const scopedJsxTransform = require("./scoped-jsx");
 const useRefRequiredInitialTransform = require("./useRef-required-initial");
@@ -41,6 +42,9 @@ const transform = (file, api, options) => {
 	if (transformNames.has("deprecated-void-function-component")) {
 		transforms.push(deprecatedVoidFunctionComponentTransform);
 	}
+	if (transformNames.has("no-implicit-ref-callback-return")) {
+		transforms.push(noImplicitRefCallbackReturnTransform);
+	}
 	if (transformNames.has("refobject-defaults")) {
 		transforms.push(refobjectDefaultsTransform);
 	}