@samitouri / QOS-React-1 / commits / 41b164ed24

[validation] Runtime validation for hook calls

--- I modeled guards as try-finally blocks to be extremely explicit. An alternative implementation could flatten all nested hooks and only set / restore hook guards when entering / exiting a React function (i.e. hook or component) -- this alternative approach would be the easiest to represent as a separate pass ```js // source function Foo() { const result = useHook(useContext(Context)); ... } // current output function Foo() { try { pushHookGuard(); const result = (() => { try { pushEnableHook(); return useHook((() => { try { pushEnableHook(); return useContext(Context); } finally { popEnableHook(); } })()); } finally { popEnableHook(); }; })(); // ... } finally { popHookGuard(); } } // alternative output function Foo() { try { // check current is not lazyDispatcher; // save originalDispatcher, set lazyDispatcher pushHookGuard(); allowHook(); // always set originalDispatcher const t0 = useContext(Context); disallowHook(); // always set LazyDispatcher allowHook(); // always set originalDispatcher const result = useHook(t0); disallowHook(); // always set LazyDispatcher // ... } finally { popHookGuard(); // restore originalDispatcher } } ``` Checked that IG Web works as expected Unless I add a sneaky useState: <img width="705" alt="Screenshot 2023-12-05 at 6 44 59 PM" src="https://github.com/facebook/react-forget/assets/34200447/3790bd76-7d71-44b5-a62e-f53256fb5736">

Mofei Zhang committed Dec 5, 2023 at 15:02 UTC 41b164ed248d7cd423aa79eca007ba90a167cceb
10 files changed +408 -16
compiler/packages/babel-plugin-react-forget/src/Entrypoint/Program.ts
+7
@@ -312,6 +312,13 @@ export function compileProgram(
312 );
313 externalFunctions.push(enableEmitFreeze);
314 }
315 +
316 + if (options.environment?.enableEmitHookGuards != null) {
317 + const enableEmitHookGuards = tryParseExternalFunction(
318 + options.environment.enableEmitHookGuards
319 + );
320 + externalFunctions.push(enableEmitHookGuards);
321 + }
322 } catch (err) {
323 handleError(err, pass, null);
324 return;
compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts
+2
@@ -191,6 +191,8 @@ const EnvironmentConfigSchema = z.object({
191 */
192 enableEmitFreeze: ExternalFunctionSchema.nullish(),
193
194 + enableEmitHookGuards: ExternalFunctionSchema.nullish(),
195 +
196 /*
197 * Enables instrumentation codegen. This emits a dev-mode only call to an
198 * instrumentation function, for components and hooks that Forget compiles.
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+103 -4
@@ -8,7 +8,7 @@
8 import * as t from "@babel/types";
9 import { pruneUnusedLValues, pruneUnusedLabels, renameVariables } from ".";
10 import { CompilerError, ErrorSeverity } from "../CompilerError";
11 -import { Environment } from "../HIR";
11 +import { Environment, EnvironmentConfig, ExternalFunction } from "../HIR";
12 import {
13 BlockId,
14 GeneratedSource,
@@ -30,10 +30,12 @@ import {
30 ReactiveValue,
31 SourceLocation,
32 SpreadPattern,
33 + getHookKind,
34 } from "../HIR/HIR";
35 import { printPlace } from "../HIR/PrintHIR";
36 import { eachPatternOperand } from "../HIR/visitors";
37 import { Err, Ok, Result } from "../Utils/Result";
38 +import { GuardKind } from "../Utils/RuntimeDiagnosticConstants";
39 import { assertExhaustive } from "../Utils/utils";
40 import { buildReactiveFunction } from "./BuildReactiveFunction";
41 import { SINGLE_CHILD_FBT_TAGS } from "./MemoizeFbtOperandsInSameScope";
@@ -86,6 +88,18 @@ export function codegenFunction(
88 );
89 compiled.body.body.unshift(test);
90 }
91 +
92 + const hookGuard = fn.env.config.enableEmitHookGuards;
93 + if (hookGuard != null) {
94 + compiled.body = t.blockStatement([
95 + createHookGuard(
96 + hookGuard,
97 + compiled.body.body,
98 + GuardKind.PushHookGuard,
99 + GuardKind.PopHookGuard
100 + ),
101 + ]);
102 + }
103 return compileResult;
104 }
105
@@ -970,7 +984,6 @@ function withLoc<T extends (...args: any[]) => t.Node>(
984 }
985
986 const createBinaryExpression = withLoc(t.binaryExpression);
973 -const createCallExpression = withLoc(t.callExpression);
987 const createExpressionStatement = withLoc(t.expressionStatement);
988 const _createLabelledStatement = withLoc(t.labeledStatement);
989 const createVariableDeclaration = withLoc(t.variableDeclaration);
@@ -989,6 +1002,77 @@ const createJsxText = withLoc(t.jsxText);
1002 const createJsxClosingElement = withLoc(t.jsxClosingElement);
1003 const createStringLiteral = withLoc(t.stringLiteral);
1004
1005 +function createHookGuard(
1006 + guard: ExternalFunction,
1007 + stmts: t.Statement[],
1008 + before: GuardKind,
1009 + after: GuardKind
1010 +): t.TryStatement {
1011 + function createHookGuardImpl(kind: number): t.ExpressionStatement {
1012 + return t.expressionStatement(
1013 + t.callExpression(t.identifier(guard.importSpecifierName), [
1014 + t.numericLiteral(kind),
1015 + ])
1016 + );
1017 + }
1018 +
1019 + return t.tryStatement(
1020 + t.blockStatement([createHookGuardImpl(before), ...stmts]),
1021 + null,
1022 + t.blockStatement([createHookGuardImpl(after)])
1023 + );
1024 +}
1025 +
1026 +/**
1027 + * Create a call expression.
1028 + * If enableEmitHookGuards is set and the callExpression is a hook call,
1029 + * the following transform will be made.
1030 + * ```js
1031 + * // source
1032 + * useHook(arg1, arg2)
1033 + *
1034 + * // codegen
1035 + * (() => {
1036 + * try {
1037 + * $dispatcherGuard(PUSH_EXPECT_HOOK);
1038 + * return useHook(arg1, arg2);
1039 + * } finally {
1040 + * $dispatcherGuard(POP_EXPECT_HOOK);
1041 + * }
1042 + * })()
1043 + * ```
1044 + */
1045 +function createCallExpression(
1046 + config: EnvironmentConfig,
1047 + callee: t.Expression,
1048 + args: Array<t.Expression | t.SpreadElement>,
1049 + loc: SourceLocation | null,
1050 + isHook: boolean
1051 +): t.CallExpression {
1052 + const callExpr = t.callExpression(callee, args);
1053 + if (loc != null && loc != GeneratedSource) {
1054 + callExpr.loc = loc;
1055 + }
1056 +
1057 + const hookGuard = config.enableEmitHookGuards;
1058 + if (hookGuard != null && isHook) {
1059 + const iife = t.arrowFunctionExpression(
1060 + [],
1061 + t.blockStatement([
1062 + createHookGuard(
1063 + hookGuard,
1064 + [t.returnStatement(callExpr)],
1065 + GuardKind.AllowHook,
1066 + GuardKind.DisallowHook
1067 + ),
1068 + ])
1069 + );
1070 + return t.callExpression(iife, []);
1071 + } else {
1072 + return callExpr;
1073 + }
1074 +}
1075 +
1076 type Temporaries = Map<IdentifierId, t.Expression | t.JSXText | null>;
1077
1078 function codegenLabel(id: BlockId): string {
@@ -1091,9 +1175,16 @@ function codegenInstructionValue(
1175 break;
1176 }
1177 case "CallExpression": {
1178 + const isHook = getHookKind(cx.env, instrValue.callee.identifier) != null;
1179 const callee = codegenPlaceToExpression(cx, instrValue.callee);
1180 const args = instrValue.args.map((arg) => codegenArgument(cx, arg));
1096 - value = createCallExpression(instrValue.loc, callee, args);
1181 + value = createCallExpression(
1182 + cx.env.config,
1183 + callee,
1184 + args,
1185 + instrValue.loc,
1186 + isHook
1187 + );
1188 break;
1189 }
1190 case "OptionalExpression": {
@@ -1147,6 +1238,8 @@ function codegenInstructionValue(
1238 break;
1239 }
1240 case "MethodCall": {
1241 + const isHook =
1242 + getHookKind(cx.env, instrValue.property.identifier) != null;
1243 const memberExpr = codegenPlaceToExpression(cx, instrValue.property);
1244 CompilerError.invariant(
1245 t.isMemberExpression(memberExpr) ||
@@ -1175,7 +1268,13 @@ function codegenInstructionValue(
1268 }
1269 );
1270 const args = instrValue.args.map((arg) => codegenArgument(cx, arg));
1178 - value = createCallExpression(instrValue.loc, memberExpr, args);
1271 + value = createCallExpression(
1272 + cx.env.config,
1273 + memberExpr,
1274 + args,
1275 + instrValue.loc,
1276 + isHook
1277 + );
1278 break;
1279 }
1280 case "NewExpression": {
compiler/packages/babel-plugin-react-forget/src/Utils/RuntimeDiagnosticConstants.ts new
+14
@@ -0,0 +1,14 @@
1 +/*
2 + * Copyright (c) Meta Platforms, Inc. and affiliates.
3 + *
4 + * This source code is licensed under the MIT license found in the
5 + * LICENSE file in the root directory of this source tree.
6 + */
7 +
8 +// WARNING: ensure this is synced with enum values in react-forget-runtime:GuardKind
9 +export enum GuardKind {
10 + PushHookGuard = 0,
11 + PopHookGuard = 1,
12 + AllowHook = 2,
13 + DisallowHook = 3,
14 +}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/flag-enable-emit-hook-guards.expect.md new
+132
@@ -0,0 +1,132 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @enableEmitHookGuards
6 +import { createContext, useContext, useEffect, useState } from "react";
7 +import {
8 + CONST_STRING0,
9 + ObjectWithHooks,
10 + getNumber,
11 + identity,
12 + print,
13 +} from "shared-runtime";
14 +
15 +const MyContext = createContext("my context value");
16 +function Component({ value }) {
17 + print(identity(CONST_STRING0));
18 + const [state, setState] = useState(getNumber());
19 + print(value, state);
20 + useEffect(() => {
21 + if (state === 4) {
22 + setState(5);
23 + }
24 + }, [state]);
25 + print(identity(value + state));
26 + return ObjectWithHooks.useIdentity(useContext(MyContext));
27 +}
28 +
29 +export const FIXTURE_ENTRYPOINT = {
30 + fn: Component,
31 + args: [{ value: 0 }],
32 +};
33 +
34 +```
35 +
36 +## Code
37 +
38 +```javascript
39 +import { $dispatcherGuard } from "react-forget-runtime"; // @enableEmitHookGuards
40 +import {
41 + createContext,
42 + useContext,
43 + useEffect,
44 + useState,
45 + unstable_useMemoCache as useMemoCache,
46 +} from "react";
47 +import {
48 + CONST_STRING0,
49 + ObjectWithHooks,
50 + getNumber,
51 + identity,
52 + print,
53 +} from "shared-runtime";
54 +
55 +const MyContext = createContext("my context value");
56 +function Component(t47) {
57 + try {
58 + $dispatcherGuard(0);
59 + const $ = useMemoCache(4);
60 + const { value } = t47;
61 + print(identity(CONST_STRING0));
62 + let t0;
63 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
64 + t0 = getNumber();
65 + $[0] = t0;
66 + } else {
67 + t0 = $[0];
68 + }
69 + const [state, setState] = (() => {
70 + try {
71 + $dispatcherGuard(2);
72 + return useState(t0);
73 + } finally {
74 + $dispatcherGuard(3);
75 + }
76 + })();
77 + print(value, state);
78 + let t1;
79 + let t2;
80 + if ($[1] !== state) {
81 + t1 = () => {
82 + if (state === 4) {
83 + setState(5);
84 + }
85 + };
86 +
87 + t2 = [state];
88 + $[1] = state;
89 + $[2] = t1;
90 + $[3] = t2;
91 + } else {
92 + t1 = $[2];
93 + t2 = $[3];
94 + }
95 + (() => {
96 + try {
97 + $dispatcherGuard(2);
98 + return useEffect(t1, t2);
99 + } finally {
100 + $dispatcherGuard(3);
101 + }
102 + })();
103 + print(identity(value + state));
104 + return (() => {
105 + try {
106 + $dispatcherGuard(2);
107 + return ObjectWithHooks.useIdentity(
108 + (() => {
109 + try {
110 + $dispatcherGuard(2);
111 + return useContext(MyContext);
112 + } finally {
113 + $dispatcherGuard(3);
114 + }
115 + })()
116 + );
117 + } finally {
118 + $dispatcherGuard(3);
119 + }
120 + })();
121 + } finally {
122 + $dispatcherGuard(1);
123 + }
124 +}
125 +
126 +export const FIXTURE_ENTRYPOINT = {
127 + fn: Component,
128 + args: [{ value: 0 }],
129 +};
130 +
131 +```
132 +
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/flag-enable-emit-hook-guards.ts new
+28
@@ -0,0 +1,28 @@
1 +// @enableEmitHookGuards
2 +import { createContext, useContext, useEffect, useState } from "react";
3 +import {
4 + CONST_STRING0,
5 + ObjectWithHooks,
6 + getNumber,
7 + identity,
8 + print,
9 +} from "shared-runtime";
10 +
11 +const MyContext = createContext("my context value");
12 +function Component({ value }) {
13 + print(identity(CONST_STRING0));
14 + const [state, setState] = useState(getNumber());
15 + print(value, state);
16 + useEffect(() => {
17 + if (state === 4) {
18 + setState(5);
19 + }
20 + }, [state]);
21 + print(identity(value + state));
22 + return ObjectWithHooks.useIdentity(useContext(MyContext));
23 +}
24 +
25 +export const FIXTURE_ENTRYPOINT = {
26 + fn: Component,
27 + args: [{ value: 0 }],
28 +};
compiler/packages/fixture-test-utils/src/compiler-utils.ts
+8
@@ -33,6 +33,7 @@ export function transformFixtureInput(
33 let gating = null;
34 let enableEmitInstrumentForget = null;
35 let enableEmitFreeze = null;
36 + let enableEmitHookGuards = null;
37 let compilationMode: CompilationMode = "all";
38 let enableUseMemoCachePolyfill = false;
39 let panicThreshold: PanicThresholdOptions = "ALL_ERRORS";
@@ -70,6 +71,12 @@ export function transformFixtureInput(
71 importSpecifierName: "makeReadOnly",
72 };
73 }
74 + if (firstLine.includes("@enableEmitHookGuards")) {
75 + enableEmitHookGuards = {
76 + source: "react-forget-runtime",
77 + importSpecifierName: "$dispatcherGuard",
78 + };
79 + }
80 if (firstLine.includes("@enableUseMemoCachePolyfill")) {
81 enableUseMemoCachePolyfill = true;
82 }
@@ -116,6 +123,7 @@ export function transformFixtureInput(
123 ]),
124 enableEmitFreeze,
125 enableEmitInstrumentForget,
126 + enableEmitHookGuards,
127 assertValidMutableRanges: true,
128 },
129 compilationMode,
compiler/packages/react-forget-runtime/src/index.ts
+108 -12
@@ -5,6 +5,7 @@
5 * LICENSE file in the root directory of this source tree.
6 */
7
8 +import invariant from "invariant";
9 import * as React from "react";
10
11 const {
@@ -41,7 +42,7 @@ export function $read(memoCache: MemoCache, index: number) {
42 return value;
43 }
44
44 -const LazyGuardDispatcher: { [key: string]: () => never } = {};
45 +const LazyGuardDispatcher: { [key: string]: (...args: Array<any>) => any } = {};
46 [
47 "readContext",
48 "useCallback",
@@ -66,26 +67,121 @@ const LazyGuardDispatcher: { [key: string]: () => never } = {};
67 "useCacheRefresh",
68 ].forEach((name) => {
69 LazyGuardDispatcher[name] = () => {
69 - throw new Error(`Cannot call ${name} within ReactForget lazy block.`);
70 + throw new Error(
71 + `[React] Unexpected React hook call (${name}) from a React Forget compiled function. ` +
72 + "Check that all hooks are called directly and named according to convention ('use[A-Z]') "
73 + );
74 };
75 });
76
77 let originalDispatcher: unknown = null;
78
75 -export function $startLazy() {
76 - if (originalDispatcher !== null) {
77 - throw new Error("unexpected startLazy with dispatcher set");
79 +// Allow guards are not emitted for useMemoCache
80 +LazyGuardDispatcher["useMemoCache"] = (count: number) => {
81 + if (originalDispatcher == null) {
82 + throw new Error(
83 + "React Forget internal invariant violation: unexpected null dispatcher"
84 + );
85 + } else {
86 + return (originalDispatcher as any).useMemoCache(count);
87 }
79 - originalDispatcher = ReactCurrentDispatcher.current;
80 - ReactCurrentDispatcher.current = LazyGuardDispatcher;
88 +};
89 +
90 +enum GuardKind {
91 + PushGuardContext = 0,
92 + PopGuardContext = 1,
93 + PushExpectHook = 2,
94 + PopExpectHook = 3,
95 +}
96 +
97 +function setCurrent(newDispatcher: any) {
98 + ReactCurrentDispatcher.current = newDispatcher;
99 + return ReactCurrentDispatcher.current;
100 }
101
83 -export function $endLazy() {
84 - if (originalDispatcher === null) {
85 - throw new Error("unexpected endLazy with dispatcher not set");
102 +const guardFrames: Array<unknown> = [];
103 +
104 +/**
105 + * When `enableEmitHookGuards` is set, this does runtime validation
106 + * of the no-conditional-hook-calls rule.
107 + * As Forget needs to statically understand which calls to move out of
108 + * conditional branches (i.e. Forget cannot memoize the results of hook
109 + * calls), its understanding of "the rules of React" are more restrictive.
110 + * This validation throws on unsound inputs at runtime.
111 + *
112 + * Components should only be invoked through React as Forget could memoize
113 + * the call to AnotherComponent, introducing conditional hook calls in its
114 + * compiled output.
115 + * ```js
116 + * function Invalid(props) {
117 + * const myJsx = AnotherComponent(props);
118 + * return <div> { myJsx } </div>;
119 + * }
120 + *
121 + * Hooks must be named as hooks.
122 + * ```js
123 + * const renamedHook = useState;
124 + * function Invalid() {
125 + * const [state, setState] = renamedHook(0);
126 + * }
127 + * ```
128 + *
129 + * Hooks must be directly called.
130 + * ```
131 + * function call(fn) {
132 + * return fn();
133 + * }
134 + * function Invalid() {
135 + * const result = call(useMyHook);
136 + * }
137 + * ```
138 + */
139 +export function $dispatcherGuard(kind: GuardKind) {
140 + const curr = ReactCurrentDispatcher.current;
141 + if (kind === GuardKind.PushGuardContext) {
142 + // Push before checking invariant or errors
143 + guardFrames.push(curr);
144 +
145 + if (guardFrames.length === 1) {
146 + // save if we're the first guard on the stack
147 + originalDispatcher = curr;
148 + }
149 +
150 + if (curr === LazyGuardDispatcher) {
151 + throw new Error(
152 + `[React] Unexpected call to custom hook or component from a React Forget compiled function. ` +
153 + "Check that (1) all hooks are called directly and named according to convention ('use[A-Z]') " +
154 + "and (2) components are returned as JSX instead of being directly invoked."
155 + );
156 + }
157 + setCurrent(LazyGuardDispatcher);
158 + } else if (kind === GuardKind.PopGuardContext) {
159 + // Pop before checking invariant or errors
160 + const lastFrame = guardFrames.pop();
161 +
162 + invariant(
163 + lastFrame != null,
164 + "React Forget internal error: unexpected null in guard stack"
165 + );
166 + if (guardFrames.length === 0) {
167 + originalDispatcher = null;
168 + }
169 + setCurrent(lastFrame);
170 + } else if (kind === GuardKind.PushExpectHook) {
171 + // ExpectHooks could be nested, so we save the current dispatcher
172 + // for the matching PopExpectHook to restore.
173 + guardFrames.push(curr);
174 + setCurrent(originalDispatcher);
175 + } else if (kind === GuardKind.PopExpectHook) {
176 + const lastFrame = guardFrames.pop();
177 + invariant(
178 + lastFrame != null,
179 + "React Forget internal error: unexpected null in guard stack"
180 + );
181 + setCurrent(lastFrame);
182 + } else {
183 + invariant(false, "Forget internal error: unreachable block" + kind);
184 }
87 - ReactCurrentDispatcher.current = originalDispatcher;
88 - originalDispatcher = null;
185 }
186
187 export function $reset($: MemoCache) {
compiler/packages/sprout/src/SproutTodoFilter.ts
+3
@@ -515,6 +515,9 @@ const skipFilter = new Set([
515 "bug-jsx-memberexpr-tag-in-lambda",
516 "bug-invalid-code-when-bailout",
517 "component-syntax-ref-gating.flow",
518 +
519 + // 'react-forget-runtime' not yet supported
520 + "flag-enable-emit-hook-guards",
521 ]);
522
523 export default skipFilter;
compiler/packages/sprout/src/shared-runtime.ts
+3
@@ -229,4 +229,7 @@ export const ObjectWithHooks = {
229 useMakeArray(): Array<number> {
230 return [1, 2, 3];
231 },
232 + useIdentity<T>(arg: T): T {
233 + return arg;
234 + }
235 };