@samitouri / QOS-React / commits / 2d569a3353

Add detection of dynamic hooks

ghstack-source-id: 3acfa4fde5dfd65353fa407378057ca937ee3599 Pull Request resolved: https://github.com/facebook/react-forget/pull/2901

Joe Savona committed Apr 24, 2024 at 15:15 UTC 2d569a3353012d23b025847fd70869fbd20ef6fa
18 files changed +164 -26
compiler/packages/babel-plugin-react-forget/src/Validation/ValidateHooksUsage.ts
+29 -8
@@ -122,8 +122,7 @@ export function validateHooksUsage(fn: HIRFunction): void {
122 place.loc,
123 new CompilerErrorDetail({
124 description: null,
125 - reason:
126 - "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)",
125 + reason,
126 loc: place.loc,
127 severity: ErrorSeverity.InvalidReact,
128 suggestions: null,
@@ -140,7 +139,24 @@ export function validateHooksUsage(fn: HIRFunction): void {
139 new CompilerErrorDetail({
140 description: null,
141 reason:
143 - "Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning)",
142 + "Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values",
143 + loc: place.loc,
144 + severity: ErrorSeverity.InvalidReact,
145 + suggestions: null,
146 + })
147 + );
148 + }
149 + }
150 + function recordDynamicHookUsageError(place: Place): void {
151 + const previousError =
152 + typeof place.loc !== "symbol" ? errorsByPlace.get(place.loc) : undefined;
153 + if (previousError === undefined) {
154 + recordError(
155 + place.loc,
156 + new CompilerErrorDetail({
157 + description: null,
158 + reason:
159 + "Hooks must be the same function on every render, but this value may change over time to a different function. See https://react.dev/reference/rules/react-calls-components-and-hooks#dont-dynamically-use-hooks",
160 loc: place.loc,
161 severity: ErrorSeverity.InvalidReact,
162 suggestions: null,
@@ -224,7 +240,10 @@ export function validateHooksUsage(fn: HIRFunction): void {
240 case "StoreLocal":
241 case "StoreContext": {
242 visitPlace(instr.value.value);
227 - const kind = getKindForPlace(instr.value.value);
243 + const kind = joinKinds(
244 + getKindForPlace(instr.value.value),
245 + getKindForPlace(instr.value.lvalue.place)
246 + );
247 setKind(instr.value.lvalue.place, kind);
248 setKind(instr.lvalue, kind);
249 break;
@@ -298,10 +317,11 @@ export function validateHooksUsage(fn: HIRFunction): void {
317 calleeKind === Kind.KnownHook || calleeKind === Kind.PotentialHook;
318 if (isHookCallee && !unconditionalBlocks.has(block.id)) {
319 recordConditionalHookError(instr.value.callee);
320 + } else if (calleeKind === Kind.PotentialHook) {
321 + recordDynamicHookUsageError(instr.value.callee);
322 }
323 /**
303 - * We intentionally skip the callee because known/potential hooks
304 - * are always allowed to be called.
324 + * We intentionally skip the callee because it's validated above
325 */
326 for (const operand of eachInstructionOperand(instr)) {
327 if (operand === instr.value.callee) {
@@ -317,10 +337,11 @@ export function validateHooksUsage(fn: HIRFunction): void {
337 calleeKind === Kind.KnownHook || calleeKind === Kind.PotentialHook;
338 if (isHookCallee && !unconditionalBlocks.has(block.id)) {
339 recordConditionalHookError(instr.value.property);
340 + } else if (calleeKind === Kind.PotentialHook) {
341 + recordDynamicHookUsageError(instr.value.property);
342 }
343 /*
322 - * We intentionally skip the callee because known/potential hooks
323 - * are always allowed to be called as methods (`React.useState()`).
344 + * We intentionally skip the property because it's validated above
345 */
346 for (const operand of eachInstructionOperand(instr)) {
347 if (operand === instr.value.property) {
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.hook-property-load-local-hook.expect.md
+2 -2
@@ -26,9 +26,9 @@ export const FIXTURE_ENTRYPOINT = {
26 5 |
27 6 | function Foo() {
28 > 7 | let bar = useFoo.useBar;
29 - | ^^^^^^^^^^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (7:7)
29 + | ^^^^^^^^^^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (7:7)
30
31 -InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (8:8)
31 +InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (8:8)
32 8 | return bar();
33 9 | }
34 10 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-assign-hook-to-local.expect.md
+1 -1
@@ -16,7 +16,7 @@ function Component(props) {
16 ```
17 1 | function Component(props) {
18 > 2 | const x = useState;
19 - | ^^^^^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (2:2)
19 + | ^^^^^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (2:2)
20 3 | const state = x(null);
21 4 | return state[0];
22 5 | }
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-pass-hook-as-call-arg.expect.md
+1 -1
@@ -14,7 +14,7 @@ function Component(props) {
14 ```
15 1 | function Component(props) {
16 > 2 | return foo(useFoo);
17 - | ^^^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (2:2)
17 + | ^^^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (2:2)
18 3 | }
19 4 |
20 ```
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-pass-hook-as-prop.expect.md
+1 -1
@@ -14,7 +14,7 @@ function Component(props) {
14 ```
15 1 | function Component(props) {
16 > 2 | return <Child foo={useFoo} />;
17 - | ^^^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (2:2)
17 + | ^^^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (2:2)
18 3 | }
19 4 |
20 ```
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ternary-with-hook-values.expect.md
+4 -4
@@ -15,13 +15,13 @@ function Component(props) {
15 ```
16 1 | function Component(props) {
17 > 2 | const x = props.cond ? useA : useB;
18 - | ^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (2:2)
18 + | ^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (2:2)
19
20 -InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (2:2)
20 +InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (2:2)
21
22 -InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (2:2)
22 +InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (2:2)
23
24 -InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (3:3)
24 +InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (3:3)
25 3 | return x();
26 4 | }
27 5 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.propertyload-hook.expect.md
+2 -2
@@ -15,9 +15,9 @@ function Component() {
15 ```
16 1 | function Component() {
17 > 2 | const x = Foo.useFoo;
18 - | ^^^^^^^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (2:2)
18 + | ^^^^^^^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (2:2)
19
20 -InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (3:3)
20 +InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (3:3)
21 3 | return x();
22 4 | }
23 5 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-call-phi-possibly-hook.expect.md
+3 -3
@@ -21,11 +21,11 @@ function Component(props) {
21 1 | function Component(props) {
22 2 | // This is a violation of using a hook as a normal value rule:
23 > 3 | const getUser = props.cond ? useGetUser : emptyFunction;
24 - | ^^^^^^^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (3:3)
24 + | ^^^^^^^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (3:3)
25
26 -InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (3:3)
26 +InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (3:3)
27
28 -InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (8:8)
28 +InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (8:8)
29 4 |
30 5 | // Ideally we would report a "conditional hook call" error here.
31 6 | // It's an unconditional call, but the value may or may not be a hook.
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-dynamic-hook-via-hooklike-local.expect.md new
+25
@@ -0,0 +1,25 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component() {
6 + const someFunction = useContext(FooContext);
7 + const useOhItsNamedLikeAHookNow = someFunction;
8 + useOhItsNamedLikeAHookNow();
9 +}
10 +
11 +```
12 +
13 +
14 +## Error
15 +
16 +```
17 + 2 | const someFunction = useContext(FooContext);
18 + 3 | const useOhItsNamedLikeAHookNow = someFunction;
19 +> 4 | useOhItsNamedLikeAHookNow();
20 + | ^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: Hooks must be the same function on every render, but this value may change over time to a different function. See https://react.dev/reference/rules/react-calls-components-and-hooks#dont-dynamically-use-hooks (4:4)
21 + 5 | }
22 + 6 |
23 +```
24 +
25 +
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-dynamic-hook-via-hooklike-local.js new
+5
@@ -0,0 +1,5 @@
1 +function Component() {
2 + const someFunction = useContext(FooContext);
3 + const useOhItsNamedLikeAHookNow = someFunction;
4 + useOhItsNamedLikeAHookNow();
5 +}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-as-conditional-test.expect.md
+1 -1
@@ -15,7 +15,7 @@ function Component(props) {
15 ```
16 1 | function Component(props) {
17 > 2 | const x = props.cond ? (useFoo ? 1 : 2) : 3;
18 - | ^^^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (2:2)
18 + | ^^^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (2:2)
19 3 | return x;
20 4 | }
21 5 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-as-prop.expect.md new
+22
@@ -0,0 +1,22 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component({ useFoo }) {
6 + useFoo();
7 +}
8 +
9 +```
10 +
11 +
12 +## Error
13 +
14 +```
15 + 1 | function Component({ useFoo }) {
16 +> 2 | useFoo();
17 + | ^^^^^^ InvalidReact: Hooks must be the same function on every render, but this value may change over time to a different function. See https://react.dev/reference/rules/react-calls-components-and-hooks#dont-dynamically-use-hooks (2:2)
18 + 3 | }
19 + 4 |
20 +```
21 +
22 +
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-as-prop.js new
+3
@@ -0,0 +1,3 @@
1 +function Component({ useFoo }) {
2 + useFoo();
3 +}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-from-hook-return.expect.md new
+26
@@ -0,0 +1,26 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function useFoo({ data }) {
6 + const useMedia = useVideoPlayer();
7 + const foo = useMedia();
8 + return foo;
9 +}
10 +
11 +```
12 +
13 +
14 +## Error
15 +
16 +```
17 + 1 | function useFoo({ data }) {
18 + 2 | const useMedia = useVideoPlayer();
19 +> 3 | const foo = useMedia();
20 + | ^^^^^^^^ InvalidReact: Hooks must be the same function on every render, but this value may change over time to a different function. See https://react.dev/reference/rules/react-calls-components-and-hooks#dont-dynamically-use-hooks (3:3)
21 + 4 | return foo;
22 + 5 | }
23 + 6 |
24 +```
25 +
26 +
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-from-hook-return.js new
+5
@@ -0,0 +1,5 @@
1 +function useFoo({ data }) {
2 + const useMedia = useVideoPlayer();
3 + const foo = useMedia();
4 + return foo;
5 +}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-from-property-of-other-hook.expect.md new
+26
@@ -0,0 +1,26 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function useFoo({ data }) {
6 + const player = useVideoPlayer();
7 + const foo = player.useMedia();
8 + return foo;
9 +}
10 +
11 +```
12 +
13 +
14 +## Error
15 +
16 +```
17 + 1 | function useFoo({ data }) {
18 + 2 | const player = useVideoPlayer();
19 +> 3 | const foo = player.useMedia();
20 + | ^^^^^^^^^^^^^^^ InvalidReact: Hooks must be the same function on every render, but this value may change over time to a different function. See https://react.dev/reference/rules/react-calls-components-and-hooks#dont-dynamically-use-hooks (3:3)
21 + 4 | return foo;
22 + 5 | }
23 + 6 |
24 +```
25 +
26 +
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-from-property-of-other-hook.js new
+5
@@ -0,0 +1,5 @@
1 +function useFoo({ data }) {
2 + const player = useVideoPlayer();
3 + const foo = player.useMedia();
4 + return foo;
5 +}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/rules-of-hooks/error.invalid-hook-reassigned-in-conditional.expect.md
+3 -3
@@ -17,11 +17,11 @@ function Component(props) {
17 1 | function Component(props) {
18 2 | let y;
19 > 3 | props.cond ? (y = useFoo) : null;
20 - | ^^^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (3:3)
20 + | ^^^^^^ InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (3:3)
21
22 -InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (3:3)
22 +InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (3:3)
23
24 -InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (4:4)
24 +InvalidReact: Hooks may not be referenced as normal values, they must be called. See https://react.dev/reference/rules/react-calls-components-and-hooks#never-pass-around-hooks-as-regular-values (4:4)
25 4 | return y();
26 5 | }
27 6 |