From 3653ae2de35397bcd84cda8955f4628c94f90663 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Tue, 29 Aug 2023 22:09:40 +0100 Subject: [PATCH] Option to infer React functions to compile Adds a new option to infer which functions to compile, based on React's ESLint rule. The main difference is that in addition to checking the function name we also check that it creates JSX or calls a hook. This should cover a significant majority of components and reduce the chance of accidentally targeting non-components, but it will leave some false negatives. Note that some cases that the ESLint plugin infers as React functions don't work yet: we don't compile FunctionExpressions, only ArrowFunctionExpressions, and the way we handle ArrowFunctionExpression doesn't work with things like forwardRef or variable declarations. We'll need more updates to fully handle all these cases, which I'll do later in the stack. --- .../src/Entrypoint/Options.ts | 7 + .../src/Entrypoint/Program.ts | 173 ++++++++++++++++++ ...r.todo.infer-function-assignment.expect.md | 19 ++ .../error.todo.infer-function-assignment.js | 4 + .../infer-function-React-memo.expect.md | 21 +++ .../compiler/infer-function-React-memo.js | 4 + .../infer-function-forwardRef.expect.md | 21 +++ .../compiler/infer-function-forwardRef.js | 4 + ...nctions-component-with-hook-call.expect.md | 33 ++++ ...nfer-functions-component-with-hook-call.js | 5 + ...fer-functions-component-with-jsx.expect.md | 29 +++ .../infer-functions-component-with-jsx.js | 4 + ...er-functions-hook-with-hook-call.expect.md | 33 ++++ .../infer-functions-hook-with-hook-call.js | 5 + .../infer-functions-hook-with-jsx.expect.md | 29 +++ .../compiler/infer-functions-hook-with-jsx.js | 4 + .../fixture-test-utils/src/compiler-utils.ts | 16 ++ .../packages/sprout/src/SproutTodoFilter.ts | 7 + 18 files changed, 418 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo.infer-function-assignment.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo.infer-function-assignment.js create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-function-React-memo.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-function-React-memo.js create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-function-forwardRef.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-function-forwardRef.js create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-component-with-hook-call.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-component-with-hook-call.js create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-component-with-jsx.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-component-with-jsx.js create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-hook-with-hook-call.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-hook-with-hook-call.js create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-hook-with-jsx.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-hook-with-jsx.js diff --git a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Options.ts b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Options.ts index d1850e56f0..801f43cf83 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Options.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Options.ts @@ -94,6 +94,12 @@ export type PluginOptions = { * */ enableOnlyOnReactScript: boolean; + + /** + * Enable to make Forget infer which components to compile, based on the same rules + * that React's ESLint rules use to detect components. + */ + enableInferReactFunctions: boolean; }; export type Logger = { @@ -103,6 +109,7 @@ export type Logger = { export const defaultOptions: PluginOptions = { enableOnlyOnReactScript: false, enableOnlyOnUseForgetDirective: false, + enableInferReactFunctions: false, panicOnBailout: true, environment: null, logger: null, diff --git a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Program.ts b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Program.ts index bbc0385e4f..1c86d8ea33 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Program.ts @@ -332,6 +332,11 @@ function shouldVisitNode( } } + if (pass.opts.enableInferReactFunctions) { + const isReactLike = isReactFunctionLike(fn); + return isReactLike; + } + return fn.scope.getProgramParent() === fn.scope.parent; } @@ -419,3 +424,171 @@ function buildBlockStatement( return body.node; } + +function isHookName(s: string): boolean { + return /^use[A-Z0-9]/.test(s); +} + +/** + * We consider hooks to be a hook name identifier or a member expression + * containing a hook name. + */ + +function isHook(path: NodePath): boolean { + if (path.isIdentifier()) { + return isHookName(path.node.name); + } else if ( + path.isMemberExpression() && + !path.node.computed && + isHook(path.get("property")) + ) { + const obj = path.get("object").node; + const isPascalCaseNameSpace = /^[A-Z].*/; + return obj.type === "Identifier" && isPascalCaseNameSpace.test(obj.name); + } else { + return false; + } +} + +/** + * Checks if the node is a React component name. React component names must + * always start with an uppercase letter. + */ + +function isComponentName(path: NodePath): boolean { + return path.isIdentifier() && /^[A-Z]/.test(path.node.name); +} + +function isReactFunction( + path: NodePath, + functionName: string +): boolean { + const node = path.node; + return ( + (node.type === "Identifier" && node.name === functionName) || + (node.type === "MemberExpression" && + node.object.type === "Identifier" && + node.object.name === "React" && + node.property.type === "Identifier" && + node.property.name === functionName) + ); +} + +/** + * Checks if the node is a callback argument of forwardRef. This render function + * should follow the rules of hooks. + */ + +function isForwardRefCallback(path: NodePath): boolean { + return !!( + path.parentPath.isCallExpression() && + path.parentPath.get("callee").isExpression() && + isReactFunction(path.parentPath.get("callee"), "forwardRef") + ); +} + +/** + * Checks if the node is a callback argument of React.memo. This anonymous + * functional component should follow the rules of hooks. + */ + +function isMemoCallback(path: NodePath): boolean { + return !!( + path.parentPath.isCallExpression() && + path.parentPath.get("callee").isExpression() && + isReactFunction(path.parentPath.get("callee"), "memo") + ); +} + +function isReactFunctionLike( + node: NodePath +): boolean { + const functionName = getFunctionName(node); + if (functionName !== null) { + if (!isComponentName(functionName) && !isHook(functionName)) { + return false; + } + } else if ( + node.isExpression() && + !isForwardRefCallback(node) && + !isMemoCallback(node) + ) { + return false; + } else { + return false; + } + + let invokesHooks = false; + let createsJsx = false; + node.traverse({ + JSX() { + createsJsx = true; + }, + CallExpression(call) { + const callee = call.get("callee"); + if (callee.isExpression() && isHook(callee)) { + invokesHooks = true; + } + }, + }); + + return invokesHooks || createsJsx; +} + +/** + * Gets the static name of a function AST node. For function declarations it is + * easy. For anonymous function expressions it is much harder. If you search for + * `IsAnonymousFunctionDefinition()` in the ECMAScript spec you'll find places + * where JS gives anonymous function expressions names. We roughly detect the + * same AST nodes with some exceptions to better fit our use case. + */ + +function getFunctionName( + path: NodePath +): NodePath | null { + if (path.isFunctionDeclaration()) { + const id = path.get("id"); + if (id.isIdentifier()) { + return id; + } + return null; + } + let id: NodePath | null = null; + const parent = path.parentPath; + if (parent.isVariableDeclarator() && parent.get("init").node === path.node) { + // const useHook = () => {}; + id = parent.get("id"); + } else if ( + parent.isAssignmentExpression() && + parent.get("right").node === path.node && + parent.get("operator") === "=" + ) { + // useHook = () => {}; + id = parent.get("left"); + } else if ( + parent.isProperty() && + parent.get("value").node === path.node && + !parent.get("computed") && + parent.get("key").isLVal() + ) { + // {useHook: () => {}} + // {useHook() {}} + id = parent.get("key"); + } else if ( + parent.isAssignmentPattern() && + parent.get("right").node === path.node && + !parent.get("computed") + ) { + // const {useHook = () => {}} = {}; + // ({useHook = () => {}} = {}); + // + // Kinda clowny, but we'd said we'd follow spec convention for + // `IsAnonymousFunctionDefinition()` usage. + id = parent.get("left"); + } + if (id !== null && (id.isIdentifier() || id.isMemberExpression())) { + return id; + } else { + return null; + } +} diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo.infer-function-assignment.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo.infer-function-assignment.expect.md new file mode 100644 index 0000000000..eb15b659c7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo.infer-function-assignment.expect.md @@ -0,0 +1,19 @@ + +## Input + +```javascript +// @enableInferReactFunctions +const Component = (props) => { + return
; +}; + +``` + + +## Error + +``` +Duplicate declaration "Component" (This is an error on an internal node. Probably an internal error.) +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo.infer-function-assignment.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo.infer-function-assignment.js new file mode 100644 index 0000000000..fb98a0b2b1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo.infer-function-assignment.js @@ -0,0 +1,4 @@ +// @enableInferReactFunctions +const Component = (props) => { + return
; +}; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-function-React-memo.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-function-React-memo.expect.md new file mode 100644 index 0000000000..9ba9acf7e1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-function-React-memo.expect.md @@ -0,0 +1,21 @@ + +## Input + +```javascript +// @enableInferReactFunctions +React.memo((props) => { + return
; +}); + +``` + +## Code + +```javascript +// @enableInferReactFunctions +React.memo((props) => { + return
; +}); + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-function-React-memo.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-function-React-memo.js new file mode 100644 index 0000000000..62eef7f971 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-function-React-memo.js @@ -0,0 +1,4 @@ +// @enableInferReactFunctions +React.memo((props) => { + return
; +}); diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-function-forwardRef.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-function-forwardRef.expect.md new file mode 100644 index 0000000000..0fdc3e46a9 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-function-forwardRef.expect.md @@ -0,0 +1,21 @@ + +## Input + +```javascript +// @enableInferReactFunctions +React.forwardRef((props) => { + return
; +}); + +``` + +## Code + +```javascript +// @enableInferReactFunctions +React.forwardRef((props) => { + return
; +}); + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-function-forwardRef.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-function-forwardRef.js new file mode 100644 index 0000000000..c029fba533 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-function-forwardRef.js @@ -0,0 +1,4 @@ +// @enableInferReactFunctions +React.forwardRef((props) => { + return
; +}); diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-component-with-hook-call.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-component-with-hook-call.expect.md new file mode 100644 index 0000000000..c66dcb9cd1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-component-with-hook-call.expect.md @@ -0,0 +1,33 @@ + +## Input + +```javascript +// @enableInferReactFunctions +function Component(props) { + const [state, _] = useState(null); + return [state]; +} + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; // @enableInferReactFunctions +function Component(props) { + const $ = useMemoCache(2); + const [state] = useState(null); + const c_0 = $[0] !== state; + let t0; + if (c_0) { + t0 = [state]; + $[0] = state; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-component-with-hook-call.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-component-with-hook-call.js new file mode 100644 index 0000000000..17f364cfdd --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-component-with-hook-call.js @@ -0,0 +1,5 @@ +// @enableInferReactFunctions +function Component(props) { + const [state, _] = useState(null); + return [state]; +} diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-component-with-jsx.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-component-with-jsx.expect.md new file mode 100644 index 0000000000..ef098059cf --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-component-with-jsx.expect.md @@ -0,0 +1,29 @@ + +## Input + +```javascript +// @enableInferReactFunctions +function Component(props) { + return
; +} + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; // @enableInferReactFunctions +function Component(props) { + const $ = useMemoCache(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 =
; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-component-with-jsx.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-component-with-jsx.js new file mode 100644 index 0000000000..1695d7e202 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-component-with-jsx.js @@ -0,0 +1,4 @@ +// @enableInferReactFunctions +function Component(props) { + return
; +} diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-hook-with-hook-call.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-hook-with-hook-call.expect.md new file mode 100644 index 0000000000..e942771864 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-hook-with-hook-call.expect.md @@ -0,0 +1,33 @@ + +## Input + +```javascript +// @enableInferReactFunctions +function useStateValue(props) { + const [state, _] = useState(null); + return [state]; +} + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; // @enableInferReactFunctions +function useStateValue(props) { + const $ = useMemoCache(2); + const [state] = useState(null); + const c_0 = $[0] !== state; + let t0; + if (c_0) { + t0 = [state]; + $[0] = state; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-hook-with-hook-call.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-hook-with-hook-call.js new file mode 100644 index 0000000000..3f68bf1cfe --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-hook-with-hook-call.js @@ -0,0 +1,5 @@ +// @enableInferReactFunctions +function useStateValue(props) { + const [state, _] = useState(null); + return [state]; +} diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-hook-with-jsx.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-hook-with-jsx.expect.md new file mode 100644 index 0000000000..824a9789a2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-hook-with-jsx.expect.md @@ -0,0 +1,29 @@ + +## Input + +```javascript +// @enableInferReactFunctions +function useDiv(props) { + return
; +} + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; // @enableInferReactFunctions +function useDiv(props) { + const $ = useMemoCache(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 =
; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-hook-with-jsx.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-hook-with-jsx.js new file mode 100644 index 0000000000..f9624bca6b --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/infer-functions-hook-with-jsx.js @@ -0,0 +1,4 @@ +// @enableInferReactFunctions +function useDiv(props) { + return
; +} diff --git a/compiler/packages/fixture-test-utils/src/compiler-utils.ts b/compiler/packages/fixture-test-utils/src/compiler-utils.ts index e5dc67e203..8d87bb9f77 100644 --- a/compiler/packages/fixture-test-utils/src/compiler-utils.ts +++ b/compiler/packages/fixture-test-utils/src/compiler-utils.ts @@ -33,6 +33,7 @@ export function transformFixtureInput( let validateNoSetStateInRender = true; let enableEmitFreeze = null; let enableOnlyOnReactScript = false; + let enableInferReactFunctions = false; if (firstLine.indexOf("@forgetDirective") !== -1) { enableOnlyOnUseForgetDirective = true; @@ -80,6 +81,20 @@ export function transformFixtureInput( enableOnlyOnReactScript = true; language = "flow"; } + if (firstLine.indexOf("@enableInferReactFunctions") !== -1) { + enableInferReactFunctions = true; + } + if ( + [ + enableInferReactFunctions, + enableOnlyOnReactScript, + enableOnlyOnUseForgetDirective, + ].filter((x) => x === true).length > 1 + ) { + throw new Error( + "Cannot enable more than one of @enableInferReactFunctions, @enableOnlyOnReactScript, and @enableOnlyOnUseForgetDirective at once" + ); + } return pluginFn( input, @@ -111,6 +126,7 @@ export function transformFixtureInput( }, enableOnlyOnUseForgetDirective, enableOnlyOnReactScript, + enableInferReactFunctions, logger: null, gating, instrumentForget, diff --git a/compiler/packages/sprout/src/SproutTodoFilter.ts b/compiler/packages/sprout/src/SproutTodoFilter.ts index 936085bd22..c66bd3f475 100644 --- a/compiler/packages/sprout/src/SproutTodoFilter.ts +++ b/compiler/packages/sprout/src/SproutTodoFilter.ts @@ -438,6 +438,13 @@ const skipFilter = new Set([ "rules-of-hooks/rules-of-hooks-e66a744cffbe", "rules-of-hooks/rules-of-hooks-eacfcaa6ef89", "rules-of-hooks/rules-of-hooks-fe6042f7628b", + "infer-functions-component-with-jsx", + "infer-function-forwardRef", + "infer-function-React-memo", + "infer-functions-component-with-hook-call", + "infer-functions-component-with-jsx", + "infer-functions-hook-with-hook-call", + "infer-functions-hook-with-jsx", ]); export default skipFilter;