From 3b9f33aa9be7ce1c22d1a8d22d0f88c774ba1bbf Mon Sep 17 00:00:00 2001 From: Mike Vitousek Date: Tue, 11 Jun 2024 13:42:30 -0700 Subject: [PATCH] [compiler] Expect components to have hook calls or jsx directly in body Summary: We can tighten our criteria for what is a component by requiring that a component or hook contain JSX or hook calls directly within its body, excluding nested functions . Currently, if we see them within the body anywhere -- including nested functions -- we treat it as a component if the other requirements are met. This change makes this stricter. We also now expect components (but not necessarily hooks) to have return statements, and those returns must be potential React nodes (we can reject functions that return function or object literals, for example). ghstack-source-id: 4507cc3955216c564bf257c0b81bfb551ae6ae55 Pull Request resolved: https://github.com/facebook/react/pull/29865 --- .../src/Entrypoint/Program.ts | 61 ++++++++++++++++++- .../infer-no-component-nested-jsx.expect.md | 51 ++++++++++++++++ .../compiler/infer-no-component-nested-jsx.js | 18 ++++++ .../infer-no-component-obj-return.expect.md | 43 +++++++++++++ .../compiler/infer-no-component-obj-return.js | 14 +++++ ...ion-expression-object-expression.expect.md | 15 ++--- ...d-function-expression-object-expression.js | 1 + ...lid-hook-in-nested-object-method.expect.md | 15 ++--- ...or.invalid-hook-in-nested-object-method.js | 1 + 9 files changed, 203 insertions(+), 16 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-no-component-nested-jsx.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-no-component-nested-jsx.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-no-component-obj-return.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-no-component-obj-return.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts index 085ac44f49bcb..8e5b00e6cd160 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -688,7 +688,8 @@ function getComponentOrHookLike( if (functionName !== null && isComponentName(functionName)) { let isComponent = callsHooksOrCreatesJsx(node, hookPattern) && - isValidComponentParams(node.get("params")); + isValidComponentParams(node.get("params")) && + !returnsNonNode(node); return isComponent ? "Component" : null; } else if (functionName !== null && isHook(functionName, hookPattern)) { // Hooks have hook invocations or JSX, but can take any # of arguments @@ -708,12 +709,31 @@ function getComponentOrHookLike( return null; } +function skipNestedFunctions( + node: NodePath< + t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression + > +) { + return ( + fn: NodePath< + t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression + > + ): void => { + if (fn.node !== node.node) { + fn.skip(); + } + }; +} + function callsHooksOrCreatesJsx( - node: NodePath, + node: NodePath< + t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression + >, hookPattern: string | null ): boolean { let invokesHooks = false; let createsJsx = false; + node.traverse({ JSX() { createsJsx = true; @@ -724,11 +744,48 @@ function callsHooksOrCreatesJsx( invokesHooks = true; } }, + ArrowFunctionExpression: skipNestedFunctions(node), + FunctionExpression: skipNestedFunctions(node), + FunctionDeclaration: skipNestedFunctions(node), }); return invokesHooks || createsJsx; } +function returnsNonNode( + node: NodePath< + t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression + > +): boolean { + let hasReturn = false; + let returnsNonNode = false; + + node.traverse({ + ReturnStatement(ret) { + hasReturn = true; + const argument = ret.node.argument; + if (argument == null) { + returnsNonNode = true; + } else { + switch (argument.type) { + case "ObjectExpression": + case "ArrowFunctionExpression": + case "FunctionExpression": + case "BigIntLiteral": + case "ClassExpression": + case "NewExpression": // technically `new Array()` is legit, but unlikely + returnsNonNode = true; + } + } + }, + ArrowFunctionExpression: skipNestedFunctions(node), + FunctionExpression: skipNestedFunctions(node), + FunctionDeclaration: skipNestedFunctions(node), + }); + + return !hasReturn || returnsNonNode; +} + /* * 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 diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-no-component-nested-jsx.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-no-component-nested-jsx.expect.md new file mode 100644 index 0000000000000..adb9a1da40ea8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-no-component-nested-jsx.expect.md @@ -0,0 +1,51 @@ + +## Input + +```javascript +// @compilationMode(infer) +function Component(props) { + const result = f(props); + function helper() { + return ; + } + helper(); + return result; +} + +function f(props) { + return props; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +## Code + +```javascript +// @compilationMode(infer) +function Component(props) { + const result = f(props); + function helper() { + return ; + } + helper(); + return result; +} + +function f(props) { + return props; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +### Eval output +(kind: ok) {} \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-no-component-nested-jsx.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-no-component-nested-jsx.js new file mode 100644 index 0000000000000..9d9e070ca9804 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-no-component-nested-jsx.js @@ -0,0 +1,18 @@ +// @compilationMode(infer) +function Component(props) { + const result = f(props); + function helper() { + return ; + } + helper(); + return result; +} + +function f(props) { + return props; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-no-component-obj-return.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-no-component-obj-return.expect.md new file mode 100644 index 0000000000000..0edadfae9ccaf --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-no-component-obj-return.expect.md @@ -0,0 +1,43 @@ + +## Input + +```javascript +// @compilationMode(infer) +function Component(props) { + const ignore = ; + return { foo: f(props) }; +} + +function f(props) { + return props; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +## Code + +```javascript +// @compilationMode(infer) +function Component(props) { + const ignore = ; + return { foo: f(props) }; +} + +function f(props) { + return props; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +### Eval output +(kind: ok) {"foo":{}} \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-no-component-obj-return.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-no-component-obj-return.js new file mode 100644 index 0000000000000..de3d8434494a8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-no-component-obj-return.js @@ -0,0 +1,14 @@ +// @compilationMode(infer) +function Component(props) { + const ignore = ; + return { foo: f(props) }; +} + +function f(props) { + return props; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-in-nested-function-expression-object-expression.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-in-nested-function-expression-object-expression.expect.md index 09d37fdaa38c6..efd736980997e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-in-nested-function-expression-object-expression.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-in-nested-function-expression-object-expression.expect.md @@ -4,6 +4,7 @@ ```javascript // @compilationMode(infer) function Component() { + "use memo"; const f = () => { const x = { outer() { @@ -27,13 +28,13 @@ function Component() { ## Error ``` - 7 | const y = { - 8 | inner() { -> 9 | return useFoo(); - | ^^^^^^ InvalidReact: Hooks must be called at the top level in the body of a function component or custom hook, and may not be called within function expressions. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning). Cannot call Custom within a function component (9:9) - 10 | }, - 11 | }; - 12 | return y; + 8 | const y = { + 9 | inner() { +> 10 | return useFoo(); + | ^^^^^^ InvalidReact: Hooks must be called at the top level in the body of a function component or custom hook, and may not be called within function expressions. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning). Cannot call Custom within a function component (10:10) + 11 | }, + 12 | }; + 13 | return y; ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-in-nested-function-expression-object-expression.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-in-nested-function-expression-object-expression.js index aead5b5e22778..981cfdade0efc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-in-nested-function-expression-object-expression.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-in-nested-function-expression-object-expression.js @@ -1,5 +1,6 @@ // @compilationMode(infer) function Component() { + "use memo"; const f = () => { const x = { outer() { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-in-nested-object-method.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-in-nested-object-method.expect.md index 56c89fa5fbcf5..bac804c00dfaf 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-in-nested-object-method.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-in-nested-object-method.expect.md @@ -4,6 +4,7 @@ ```javascript // @compilationMode(infer) function Component() { + "use memo"; const x = { outer() { const y = { @@ -23,13 +24,13 @@ function Component() { ## Error ``` - 5 | const y = { - 6 | inner() { -> 7 | return useFoo(); - | ^^^^^^ InvalidReact: Hooks must be called at the top level in the body of a function component or custom hook, and may not be called within function expressions. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning). Cannot call Custom within a function component (7:7) - 8 | }, - 9 | }; - 10 | return y; + 6 | const y = { + 7 | inner() { +> 8 | return useFoo(); + | ^^^^^^ InvalidReact: Hooks must be called at the top level in the body of a function component or custom hook, and may not be called within function expressions. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning). Cannot call Custom within a function component (8:8) + 9 | }, + 10 | }; + 11 | return y; ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-in-nested-object-method.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-in-nested-object-method.js index 5ba5a74c0de41..4d0a181fb4ca9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-in-nested-object-method.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-in-nested-object-method.js @@ -1,5 +1,6 @@ // @compilationMode(infer) function Component() { + "use memo"; const x = { outer() { const y = {