Disallow calling hooks in functions

> Don’t call Hooks inside loops, conditions, or nested functions 

Per https://react.dev/warnings/invalid-hook-call-warning#breaking-rules-of-hooks 
it is invalid to call hooks inside function expressions. We now validate this by 
default, i'll verify internally before landing. 

Note the validation is somewhat more conservative and we only disallow known 
hook calls here, this seems like a reasonable tradeoff but i'm open to 
suggestions. We could reuse the same known/potential hook mechanism here but it 
would take some more refactoring.
This commit is contained in:
Joe Savona
2024-02-16 11:00:56 -08:00
parent a4234bcfbf
commit 9c419dfd80
33 changed files with 328 additions and 410 deletions
@@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/
import * as t from "@babel/types";
import {
CompilerError,
CompilerErrorDetail,
@@ -89,21 +90,35 @@ function joinKinds(a: Kind, b: Kind): Kind {
export function validateHooksUsage(fn: HIRFunction): void {
const unconditionalBlocks = computeUnconditionalBlocks(fn);
const errorsByPlace = new Map<SourceLocation, CompilerErrorDetail>();
const errors = new CompilerError();
const errorsByPlace = new Map<t.SourceLocation, CompilerErrorDetail>();
function recordError(
loc: SourceLocation,
errorDetail: CompilerErrorDetail
): void {
if (typeof loc === "symbol") {
errors.pushErrorDetail(errorDetail);
} else {
errorsByPlace.set(loc, errorDetail);
}
}
function recordConditionalHookError(place: Place): void {
// Once a particular hook has a conditional call error, don't report any further issues for this hook
setKind(place, Kind.Error);
const reason =
"Hooks must always be called in a consistent order, and may not be called conditionally. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning)";
const previousError = errorsByPlace.get(place.loc);
const previousError =
typeof place.loc !== "symbol" ? errorsByPlace.get(place.loc) : undefined;
/*
* In some circumstances such as optional calls, we may first encounter a "hook may not be referenced as normal values" error.
* If that same place is also used as a conditional call, upgrade the error to a conditonal hook error
*/
if (previousError === undefined || previousError.reason !== reason) {
errorsByPlace.set(
recordError(
place.loc,
new CompilerErrorDetail({
description: null,
@@ -117,8 +132,10 @@ export function validateHooksUsage(fn: HIRFunction): void {
}
}
function recordInvalidHookUsageError(place: Place): void {
if (!errorsByPlace.has(place.loc)) {
errorsByPlace.set(
const previousError =
typeof place.loc !== "symbol" ? errorsByPlace.get(place.loc) : undefined;
if (previousError === undefined) {
recordError(
place.loc,
new CompilerErrorDetail({
description: null,
@@ -348,6 +365,11 @@ export function validateHooksUsage(fn: HIRFunction): void {
}
break;
}
case "ObjectMethod":
case "FunctionExpression": {
visitFunctionExpression(errors, instr.value.loweredFunc.func);
break;
}
default: {
/*
* Else check usages of operands, but do *not* flow properties
@@ -369,7 +391,6 @@ export function validateHooksUsage(fn: HIRFunction): void {
}
}
const errors = new CompilerError();
for (const [, error] of errorsByPlace) {
errors.push(error);
}
@@ -377,3 +398,37 @@ export function validateHooksUsage(fn: HIRFunction): void {
throw errors;
}
}
function visitFunctionExpression(errors: CompilerError, fn: HIRFunction): void {
for (const [, block] of fn.body.blocks) {
for (const instr of block.instructions) {
switch (instr.value.kind) {
case "FunctionExpression": {
visitFunctionExpression(errors, instr.value.loweredFunc.func);
break;
}
case "MethodCall":
case "CallExpression": {
const callee =
instr.value.kind === "CallExpression"
? instr.value.callee
: instr.value.property;
const hookKind = getHookKind(fn.env, callee.identifier);
if (hookKind != null) {
errors.pushErrorDetail(
new CompilerErrorDetail({
severity: ErrorSeverity.InvalidReact,
reason:
"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)",
loc: callee.loc,
description: `Cannot call ${hookKind} within a function component`,
suggestions: null,
})
);
}
break;
}
}
}
}
}
@@ -0,0 +1,32 @@
## Input
```javascript
// @skip
// Unsupported input
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
const ComponentWithHookInsideCallback = React.forwardRef((props, ref) => {
useEffect(() => {
useHookInsideCallback();
});
return <button {...props} ref={ref} />;
});
```
## Error
```
6 | const ComponentWithHookInsideCallback = React.forwardRef((props, ref) => {
7 | useEffect(() => {
> 8 | useHookInsideCallback();
| ^^^^^^^^^^^^^^^^^^^^^ [ReactForget] 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 | return <button {...props} ref={ref} />;
11 | });
```
@@ -0,0 +1,32 @@
## Input
```javascript
// @skip
// Unsupported input
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
const ComponentWithHookInsideCallback = React.memo((props) => {
useEffect(() => {
useHookInsideCallback();
});
return <button {...props} />;
});
```
## Error
```
6 | const ComponentWithHookInsideCallback = React.memo((props) => {
7 | useEffect(() => {
> 8 | useHookInsideCallback();
| ^^^^^^^^^^^^^^^^^^^^^ [ReactForget] 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 | return <button {...props} />;
11 | });
```
@@ -0,0 +1,30 @@
## Input
```javascript
// Invalid because it's dangerous and might not warn otherwise.
// This *must* be invalid.
function createHook() {
return function useHookWithConditionalHook() {
if (cond) {
useConditionalHook();
}
};
}
```
## Error
```
4 | return function useHookWithConditionalHook() {
5 | if (cond) {
> 6 | useConditionalHook();
| ^^^^^^^^^^^^^^^^^^ [ReactForget] 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 (6:6)
7 | }
8 | };
9 | }
```
@@ -1,6 +1,3 @@
// @skip
// Passed but should have failed
// Invalid because it's dangerous and might not warn otherwise.
// This *must* be invalid.
function createHook() {
@@ -0,0 +1,32 @@
## Input
```javascript
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
function createComponent() {
return function ComponentWithHookInsideCallback() {
useEffect(() => {
useHookInsideCallback();
});
};
}
```
## Error
```
4 | return function ComponentWithHookInsideCallback() {
5 | useEffect(() => {
> 6 | useHookInsideCallback();
| ^^^^^^^^^^^^^^^^^^^^^ [ReactForget] 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 (6:6)
[ReactForget] 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 useEffect within a function component (5:5)
7 | });
8 | };
9 | }
```
@@ -1,6 +1,3 @@
// @skip
// Passed but should have failed
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
function createComponent() {
@@ -0,0 +1,30 @@
## Input
```javascript
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
function createComponent() {
return function ComponentWithHookInsideCallback() {
function handleClick() {
useState();
}
};
}
```
## Error
```
4 | return function ComponentWithHookInsideCallback() {
5 | function handleClick() {
> 6 | useState();
| ^^^^^^^^ [ReactForget] 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 useState within a function component (6:6)
7 | }
8 | };
9 | }
```
@@ -1,6 +1,3 @@
// @skip
// Passed but should have failed
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
function createComponent() {
@@ -0,0 +1,28 @@
## Input
```javascript
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
function ComponentWithHookInsideCallback() {
function handleClick() {
useState();
}
}
```
## Error
```
3 | function ComponentWithHookInsideCallback() {
4 | function handleClick() {
> 5 | useState();
| ^^^^^^^^ [ReactForget] 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 useState within a function component (5:5)
6 | }
7 | }
8 |
```
@@ -1,6 +1,3 @@
// @skip
// Passed but should have failed
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
function ComponentWithHookInsideCallback() {
@@ -0,0 +1,30 @@
## Input
```javascript
// Invalid because it's dangerous and might not warn otherwise.
// This *must* be invalid.
function createComponent() {
return function ComponentWithConditionalHook() {
if (cond) {
useConditionalHook();
}
};
}
```
## Error
```
4 | return function ComponentWithConditionalHook() {
5 | if (cond) {
> 6 | useConditionalHook();
| ^^^^^^^^^^^^^^^^^^ [ReactForget] 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 (6:6)
7 | }
8 | };
9 | }
```
@@ -1,6 +1,3 @@
// @skip
// Passed but should have failed
// Invalid because it's dangerous and might not warn otherwise.
// This *must* be invalid.
function createComponent() {
@@ -0,0 +1,28 @@
## Input
```javascript
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
function ComponentWithHookInsideCallback() {
useEffect(() => {
useHookInsideCallback();
});
}
```
## Error
```
3 | function ComponentWithHookInsideCallback() {
4 | useEffect(() => {
> 5 | useHookInsideCallback();
| ^^^^^^^^^^^^^^^^^^^^^ [ReactForget] 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 (5:5)
6 | });
7 | }
8 |
```
@@ -1,6 +1,3 @@
// @skip
// Passed but should have failed
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
function ComponentWithHookInsideCallback() {
@@ -2,6 +2,7 @@
## Input
```javascript
// @compilationMode(infer)
// Regression test for some internal code.
// This shows how the "callback rule" is more relaxed,
// and doesn't kick in unless we're confident we're in
@@ -19,6 +20,7 @@ function makeListener(instance) {
## Code
```javascript
// @compilationMode(infer)
// Regression test for some internal code.
// This shows how the "callback rule" is more relaxed,
// and doesn't kick in unless we're confident we're in
@@ -1,3 +1,4 @@
// @compilationMode(infer)
// Regression test for some internal code.
// This shows how the "callback rule" is more relaxed,
// and doesn't kick in unless we're confident we're in
@@ -2,6 +2,7 @@
## Input
```javascript
// @compilationMode(infer)
// Valid because hooks can call hooks.
function createHook() {
return function useHook() {
@@ -15,20 +16,13 @@ function createHook() {
## Code
```javascript
import { unstable_useMemoCache as useMemoCache } from "react"; // Valid because hooks can call hooks.
// @compilationMode(infer)
// Valid because hooks can call hooks.
function createHook() {
const $ = useMemoCache(1);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = function useHook() {
useHook1();
useHook2();
};
$[0] = t0;
} else {
t0 = $[0];
}
return t0;
return function useHook() {
useHook1();
useHook2();
};
}
```
@@ -1,3 +1,4 @@
// @compilationMode(infer)
// Valid because hooks can call hooks.
function createHook() {
return function useHook() {
@@ -2,6 +2,7 @@
## Input
```javascript
// @compilationMode(infer)
// Valid because hooks can use hooks.
function createHook() {
return function useHookWithHook() {
@@ -14,19 +15,12 @@ function createHook() {
## Code
```javascript
import { unstable_useMemoCache as useMemoCache } from "react"; // Valid because hooks can use hooks.
// @compilationMode(infer)
// Valid because hooks can use hooks.
function createHook() {
const $ = useMemoCache(1);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = function useHookWithHook() {
useHook();
};
$[0] = t0;
} else {
t0 = $[0];
}
return t0;
return function useHookWithHook() {
useHook();
};
}
```
@@ -1,3 +1,4 @@
// @compilationMode(infer)
// Valid because hooks can use hooks.
function createHook() {
return function useHookWithHook() {
@@ -2,6 +2,7 @@
## Input
```javascript
// @compilationMode(infer)
// Valid because components can use hooks.
function createComponentWithHook() {
return function ComponentWithHook() {
@@ -14,19 +15,12 @@ function createComponentWithHook() {
## Code
```javascript
import { unstable_useMemoCache as useMemoCache } from "react"; // Valid because components can use hooks.
// @compilationMode(infer)
// Valid because components can use hooks.
function createComponentWithHook() {
const $ = useMemoCache(1);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = function ComponentWithHook() {
useHook();
};
$[0] = t0;
} else {
t0 = $[0];
}
return t0;
return function ComponentWithHook() {
useHook();
};
}
```
@@ -1,3 +1,4 @@
// @compilationMode(infer)
// Valid because components can use hooks.
function createComponentWithHook() {
return function ComponentWithHook() {
@@ -1,52 +0,0 @@
## Input
```javascript
// @skip
// Unsupported input
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
const ComponentWithHookInsideCallback = React.forwardRef((props, ref) => {
useEffect(() => {
useHookInsideCallback();
});
return <button {...props} ref={ref} />;
});
```
## Code
```javascript
import { unstable_useMemoCache as useMemoCache } from "react"; // @skip
// Unsupported input
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
const ComponentWithHookInsideCallback = React.forwardRef((props, ref) => {
const $ = useMemoCache(4);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = () => {
useHookInsideCallback();
};
$[0] = t0;
} else {
t0 = $[0];
}
useEffect(t0);
let t1;
if ($[1] !== props || $[2] !== ref) {
t1 = <button {...props} ref={ref} />;
$[1] = props;
$[2] = ref;
$[3] = t1;
} else {
t1 = $[3];
}
return t1;
});
```
@@ -1,51 +0,0 @@
## Input
```javascript
// @skip
// Unsupported input
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
const ComponentWithHookInsideCallback = React.memo((props) => {
useEffect(() => {
useHookInsideCallback();
});
return <button {...props} />;
});
```
## Code
```javascript
import { unstable_useMemoCache as useMemoCache } from "react"; // @skip
// Unsupported input
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
const ComponentWithHookInsideCallback = React.memo((props) => {
const $ = useMemoCache(3);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = () => {
useHookInsideCallback();
};
$[0] = t0;
} else {
t0 = $[0];
}
useEffect(t0);
let t1;
if ($[1] !== props) {
t1 = <button {...props} />;
$[1] = props;
$[2] = t1;
} else {
t1 = $[2];
}
return t1;
});
```
@@ -1,45 +0,0 @@
## Input
```javascript
// @skip
// Passed but should have failed
// Invalid because it's dangerous and might not warn otherwise.
// This *must* be invalid.
function createHook() {
return function useHookWithConditionalHook() {
if (cond) {
useConditionalHook();
}
};
}
```
## Code
```javascript
import { unstable_useMemoCache as useMemoCache } from "react"; // @skip
// Passed but should have failed
// Invalid because it's dangerous and might not warn otherwise.
// This *must* be invalid.
function createHook() {
const $ = useMemoCache(1);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = function useHookWithConditionalHook() {
if (cond) {
useConditionalHook();
}
};
$[0] = t0;
} else {
t0 = $[0];
}
return t0;
}
```
@@ -1,45 +0,0 @@
## Input
```javascript
// @skip
// Passed but should have failed
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
function createComponent() {
return function ComponentWithHookInsideCallback() {
useEffect(() => {
useHookInsideCallback();
});
};
}
```
## Code
```javascript
import { unstable_useMemoCache as useMemoCache } from "react"; // @skip
// Passed but should have failed
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
function createComponent() {
const $ = useMemoCache(1);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = function ComponentWithHookInsideCallback() {
useEffect(() => {
useHookInsideCallback();
});
};
$[0] = t0;
} else {
t0 = $[0];
}
return t0;
}
```
@@ -1,41 +0,0 @@
## Input
```javascript
// @skip
// Passed but should have failed
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
function createComponent() {
return function ComponentWithHookInsideCallback() {
function handleClick() {
useState();
}
};
}
```
## Code
```javascript
import { unstable_useMemoCache as useMemoCache } from "react"; // @skip
// Passed but should have failed
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
function createComponent() {
const $ = useMemoCache(1);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = function ComponentWithHookInsideCallback() {};
$[0] = t0;
} else {
t0 = $[0];
}
return t0;
}
```
@@ -1,29 +0,0 @@
## Input
```javascript
// @skip
// Passed but should have failed
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
function ComponentWithHookInsideCallback() {
function handleClick() {
useState();
}
}
```
## Code
```javascript
// @skip
// Passed but should have failed
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
function ComponentWithHookInsideCallback() {}
```
@@ -1,45 +0,0 @@
## Input
```javascript
// @skip
// Passed but should have failed
// Invalid because it's dangerous and might not warn otherwise.
// This *must* be invalid.
function createComponent() {
return function ComponentWithConditionalHook() {
if (cond) {
useConditionalHook();
}
};
}
```
## Code
```javascript
import { unstable_useMemoCache as useMemoCache } from "react"; // @skip
// Passed but should have failed
// Invalid because it's dangerous and might not warn otherwise.
// This *must* be invalid.
function createComponent() {
const $ = useMemoCache(1);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = function ComponentWithConditionalHook() {
if (cond) {
useConditionalHook();
}
};
$[0] = t0;
} else {
t0 = $[0];
}
return t0;
}
```
@@ -1,41 +0,0 @@
## Input
```javascript
// @skip
// Passed but should have failed
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
function ComponentWithHookInsideCallback() {
useEffect(() => {
useHookInsideCallback();
});
}
```
## Code
```javascript
import { unstable_useMemoCache as useMemoCache } from "react"; // @skip
// Passed but should have failed
// Invalid because it's a common misunderstanding.
// We *could* make it valid but the runtime error could be confusing.
function ComponentWithHookInsideCallback() {
const $ = useMemoCache(1);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = () => {
useHookInsideCallback();
};
$[0] = t0;
} else {
t0 = $[0];
}
useEffect(t0);
}
```