From 41f52b73c2da70dbd0fa70d26b95ae1fcfcd6693 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 8 Feb 2023 14:24:14 -0800 Subject: [PATCH] Change reference effects for hooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I realized we hadn't updated InferReferenceEffects to match our latest thinking on hooks. Specifically, we will default to assuming that hooks can mutate their arguments and return mutable values — this works with our model since we don't treat hooks specially for reactive scope construction. Ie, first we figure out what variables construct together, then we create scopes, then we prune scopes that contain hooks. So changing the reference effects for hooks "just works". Note that it is helpful for our unit tests to have an example hook that we know _does_ freeze its input and return a frozen value, so i've temporarily added `useFreeze()` to the list of defined hooks. That is meant as a stopgap: the right solution is to allow some way to tell the compiler about specific custom hooks and their semantics. --- .../src/Inference/InferReferenceEffects.ts | 14 ++++- .../hir/optional-member-expression.expect.md | 11 +--- .../__tests__/fixtures/hir/timers.expect.md | 60 +++++++++++++++++++ .../src/__tests__/fixtures/hir/timers.js | 10 ++++ .../fixtures/hir/useRef-mutable.expect.md | 23 +++++++ .../__tests__/fixtures/hir/useRef-mutable.js | 5 ++ 6 files changed, 110 insertions(+), 13 deletions(-) create mode 100644 compiler/forget/src/__tests__/fixtures/hir/timers.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/hir/timers.js create mode 100644 compiler/forget/src/__tests__/fixtures/hir/useRef-mutable.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/hir/useRef-mutable.js diff --git a/compiler/forget/src/Inference/InferReferenceEffects.ts b/compiler/forget/src/Inference/InferReferenceEffects.ts index f00c4559bd..468bf28078 100644 --- a/compiler/forget/src/Inference/InferReferenceEffects.ts +++ b/compiler/forget/src/Inference/InferReferenceEffects.ts @@ -792,10 +792,18 @@ const HOOKS: Map = new Map([ "useRef", { kind: "Ref", - effectKind: Effect.Read, + effectKind: Effect.Capture, valueKind: ValueKind.Mutable, }, ], + [ + "useFreeze", + { + kind: "Ref", + effectKind: Effect.Freeze, + valueKind: ValueKind.Frozen, + }, + ], ]); type HookKind = { kind: "State" } | { kind: "Ref" } | { kind: "Custom" }; @@ -812,7 +820,7 @@ export function parseHookCall(place: Place): Hook | null { } return { kind: "Custom", - effectKind: Effect.Freeze, - valueKind: ValueKind.Frozen, + effectKind: Effect.Mutate, + valueKind: ValueKind.Mutable, }; } diff --git a/compiler/forget/src/__tests__/fixtures/hir/optional-member-expression.expect.md b/compiler/forget/src/__tests__/fixtures/hir/optional-member-expression.expect.md index cbc15b72b1..779bc76b45 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/optional-member-expression.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/optional-member-expression.expect.md @@ -16,16 +16,7 @@ function Foo(props) { ```javascript function Foo(props) { - const $ = React.unstable_useMemoCache(); - const c_0 = $[0] !== props.a; - let x; - if (c_0) { - x = bar(props.a); - $[0] = props.a; - $[1] = x; - } else { - x = $[1]; - } + const x = bar(props.a); const y = x?.b; const z = useBar(y); diff --git a/compiler/forget/src/__tests__/fixtures/hir/timers.expect.md b/compiler/forget/src/__tests__/fixtures/hir/timers.expect.md new file mode 100644 index 0000000000..d5efc6151b --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/timers.expect.md @@ -0,0 +1,60 @@ + +## Input + +```javascript +function Component(props) { + const start = performance.now(); + const now = Date.now(); + const time = performance.now() - start; + return ( +
+ rendering took {time} at {now} +
+ ); +} + +``` + +## Code + +```javascript +function Component(props) { + const $ = React.unstable_useMemoCache(); + let start; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + start = performance.now(); + $[0] = start; + } else { + start = $[0]; + } + let now; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + now = Date.now(); + $[1] = now; + } else { + now = $[1]; + } + let t0; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + t0 = performance.now(); + $[2] = t0; + } else { + t0 = $[2]; + } + const time = t0 - start; + let t1; + if ($[3] === Symbol.for("react.memo_cache_sentinel")) { + t1 = ( +
+ rendering took {time} at {now} +
+ ); + $[3] = t1; + } else { + t1 = $[3]; + } + return t1; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/hir/timers.js b/compiler/forget/src/__tests__/fixtures/hir/timers.js new file mode 100644 index 0000000000..eb226b89a8 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/timers.js @@ -0,0 +1,10 @@ +function Component(props) { + const start = performance.now(); + const now = Date.now(); + const time = performance.now() - start; + return ( +
+ rendering took {time} at {now} +
+ ); +} diff --git a/compiler/forget/src/__tests__/fixtures/hir/useRef-mutable.expect.md b/compiler/forget/src/__tests__/fixtures/hir/useRef-mutable.expect.md new file mode 100644 index 0000000000..6ba63442e1 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/useRef-mutable.expect.md @@ -0,0 +1,23 @@ + +## Input + +```javascript +function Component(props) { + const ref = useRef(null); + ref.current = props.value; + return ref.current; +} + +``` + +## Code + +```javascript +function Component(props) { + const ref = useRef(null); + ref.current = props.value; + return ref.current; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/hir/useRef-mutable.js b/compiler/forget/src/__tests__/fixtures/hir/useRef-mutable.js new file mode 100644 index 0000000000..97ce6931db --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/useRef-mutable.js @@ -0,0 +1,5 @@ +function Component(props) { + const ref = useRef(null); + ref.current = props.value; + return ref.current; +}