@samitouri / QOS-React-1 / commits / 6747d4e33c

PreserveMemo for useCallback transitively freezes function exprs

Merges `@enableTransitivelyFreezeFunctionExpressions` into the new `@enablePreserveExistingMemoizationGuarantees` mode, since they are both motivated by the same use case of preserving effect behavior by preserving existing memoization behavior. The idea is that `useCallback` has an implicit assumption: that the variables captured by the callback aren't subsequently modified. Previous PRs treated the values directly captured by the callback as frozen. But if those variables were themselves another function expression, and that expression captured a mutable value, then we wouldn't consider the freeze to be transitive: ```javascript const object = makeObject(); useHook(); // oops, hook call inside `object`'s mutable range, can't memoize object, log, or onClick! const log = () => { console.log(object) }; const onClick = useCallback(() => { log() }); maybeMutate(object); ``` However, the assumption of such code is that it _doesn't_ modify such transitively captured values. So here we merge `@enableTransitivelyFreezeFunctionExpressions` mode into the memoization-preserving mode. Now, the memoize instructions emitted for useCallback (and useMemo) will transitively freeze captured function expressions, allowing us to memoize. The flip side of this is that some code may be violating these rules. We'll rely on runtime validation to detect such cases.

Joe Savona committed Dec 15, 2023 at 15:19 UTC 6747d4e33c1ea25612f475be2153f666b55c2b93
7 files changed +48 -60
compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts
+4 -35
@@ -119,6 +119,10 @@ const EnvironmentConfigSchema = z.object({
119 * Our recommendation is to first try running your application with this flag enabled, then attempt
120 * to disable this flag and see what changes or breaks. This will mostly likely be effects that
121 * depend on referential equality, which can be refactored (TODO guide for this).
122 + *
123 + * NOTE: this mode treats freeze as a transitive operation for function expressions. This means
124 + * that if a useEffect or useCallback references a function value, that function value will be
125 + * considered frozen, and in turn all of its referenced variables will be considered frozen as well.
126 */
127 enablePreserveExistingMemoizationGuarantees: z.boolean().default(false),
128
@@ -245,41 +249,6 @@ const EnvironmentConfigSchema = z.object({
249 */
250 enableEmitInstrumentForget: ExternalFunctionSchema.nullish(),
251
248 - /*
249 - * Forget infers certain operations as "freezing" a value, such that those
250 - * values should not be subsequently mutated. By default this freeze operation
251 - * applies to the value itself and its direct aliases, but not values captured
252 - * by the value being frozen.
253 - *
254 - * In the following, passing `x` to JSX freezes it, which includes freezing `y`
255 - * and `z` which x may alias:
256 - *
257 - * ```
258 - * let x;
259 - * if (cond) {
260 - * x = y
261 - * } else {
262 - * x = z;
263 - * }
264 - * <div>{x}</div>
265 - * ```
266 - *
267 - * However, in the following example we currently only consider x itself to be
268 - * frozen, not `y` or `z`:
269 - *
270 - * ```
271 - * let y = ...;
272 - * let z = ...;
273 - * let x = () => { return [y, z]; };
274 - * <div>{x}</div>
275 - * ```
276 - *
277 - * With this flag enabled, function expression dependencies (values closed over)
278 - * are transitively frozen when the function itself is frozen. So in this case,
279 - * `y` and `z` would be frozen when `x` is frozen.
280 - */
281 - enableTransitivelyFreezeFunctionExpressions: z.boolean().default(false),
282 -
252 // Enable merging consecutive scopes that invalidate together.
253 enableMergeConsecutiveScopes: z.boolean().default(true),
254
compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts
+1 -1
@@ -354,7 +354,7 @@ class InferenceState {
354 reason: reasonSet,
355 });
356
357 - if (this.#env.config.enableTransitivelyFreezeFunctionExpressions) {
357 + if (this.#env.config.enablePreserveExistingMemoizationGuarantees) {
358 if (value.kind === "FunctionExpression") {
359 for (const operand of eachInstructionValueOperand(value)) {
360 this.reference(operand, Effect.Freeze, ValueReason.Other);
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/transitive-freeze-array.expect.md
+3 -3
@@ -2,7 +2,7 @@
2 ## Input
3
4 ```javascript
5 -// @enableTransitivelyFreezeFunctionExpressions
5 +// @enablePreserveExistingMemoizationGuarantees
6 const { mutate } = require("shared-runtime");
7
8 function Component(props) {
@@ -10,7 +10,7 @@ function Component(props) {
10 const y = {};
11 const items = [x, y];
12 items.pop();
13 - <div>{items}</div>; // note: enableTransitivelyFreezeFunctionExpressions only visits function expressions, not arrays, so this doesn't freeze x/y
13 + <div>{items}</div>; // note: enablePreserveExistingMemoizationGuarantees only visits function expressions, not arrays, so this doesn't freeze x/y
14 mutate(y); // ok! not part of `items` anymore bc of items.pop()
15 return [x, y, items];
16 }
@@ -25,7 +25,7 @@ export const FIXTURE_ENTRYPOINT = {
25 ## Code
26
27 ```javascript
28 -import { unstable_useMemoCache as useMemoCache } from "react"; // @enableTransitivelyFreezeFunctionExpressions
28 +import { unstable_useMemoCache as useMemoCache } from "react"; // @enablePreserveExistingMemoizationGuarantees
29 const { mutate } = require("shared-runtime");
30
31 function Component(props) {
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/transitive-freeze-array.js
+2 -2
@@ -1,4 +1,4 @@
1 -// @enableTransitivelyFreezeFunctionExpressions
1 +// @enablePreserveExistingMemoizationGuarantees
2 const { mutate } = require("shared-runtime");
3
4 function Component(props) {
@@ -6,7 +6,7 @@ function Component(props) {
6 const y = {};
7 const items = [x, y];
8 items.pop();
9 - <div>{items}</div>; // note: enableTransitivelyFreezeFunctionExpressions only visits function expressions, not arrays, so this doesn't freeze x/y
9 + <div>{items}</div>; // note: enablePreserveExistingMemoizationGuarantees only visits function expressions, not arrays, so this doesn't freeze x/y
10 mutate(y); // ok! not part of `items` anymore bc of items.pop()
11 return [x, y, items];
12 }
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/transitive-freeze-function-expressions.expect.md
+2 -2
@@ -2,7 +2,7 @@
2 ## Input
3
4 ```javascript
5 -// @enableTransitivelyFreezeFunctionExpressions
5 +// @enablePreserveExistingMemoizationGuarantees
6 function Component(props) {
7 const { data, loadNext, isLoadingNext } =
8 usePaginationFragment(props.key).items ?? [];
@@ -31,7 +31,7 @@ function Component(props) {
31 ## Code
32
33 ```javascript
34 -import { unstable_useMemoCache as useMemoCache } from "react"; // @enableTransitivelyFreezeFunctionExpressions
34 +import { unstable_useMemoCache as useMemoCache } from "react"; // @enablePreserveExistingMemoizationGuarantees
35 function Component(props) {
36 const $ = useMemoCache(10);
37 const { data, loadNext, isLoadingNext } =
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/transitive-freeze-function-expressions.js
+1 -1
@@ -1,4 +1,4 @@
1 -// @enableTransitivelyFreezeFunctionExpressions
1 +// @enablePreserveExistingMemoizationGuarantees
2 function Component(props) {
3 const { data, loadNext, isLoadingNext } =
4 usePaginationFragment(props.key).items ?? [];
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useCallback-call-second-function-which-captures-maybe-mutable-value-preserve-memoization.expect.md
+35 -16
@@ -49,28 +49,47 @@ import {
49 } from "shared-runtime";
50
51 function Component(props) {
52 - const $ = useMemoCache(1);
53 - const object = makeObject_Primitives();
54 -
55 - useHook();
56 -
57 - const log = () => {
58 - logValue(object);
59 - };
60 -
61 - const onClick = () => {
62 - log();
63 - };
64 -
65 - identity(object);
52 + const $ = useMemoCache(4);
53 let t0;
54 if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
68 - t0 = <div onClick={onClick} />;
55 + t0 = makeObject_Primitives();
56 $[0] = t0;
57 } else {
58 t0 = $[0];
59 }
73 - return t0;
60 + const object = t0;
61 +
62 + useHook();
63 + let t1;
64 + if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
65 + t1 = () => {
66 + logValue(object);
67 + };
68 + $[1] = t1;
69 + } else {
70 + t1 = $[1];
71 + }
72 + const log = t1;
73 + let t2;
74 + if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
75 + t2 = () => {
76 + log();
77 + };
78 + $[2] = t2;
79 + } else {
80 + t2 = $[2];
81 + }
82 + const onClick = t2;
83 +
84 + identity(object);
85 + let t3;
86 + if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
87 + t3 = <div onClick={onClick} />;
88 + $[3] = t3;
89 + } else {
90 + t3 = $[3];
91 + }
92 + return t3;
93 }
94
95 export const FIXTURE_ENTRYPOINT = {