[compiler] Inferred effect dependencies now include optional chains #33326

mofeiZ · 2025-05-21T18:10:22Z

Inferred effect dependencies now include optional chains.

This is a temporary solution while #32099 and its followups are worked on. Ideally, we should model reactive scope dependencies in the IR similarly to ComputeIR -- dependencies should be hoisted and all references rewritten to use the hoisted dependencies.

`

Stack created with Sapling. Best reviewed with ReviewStack.

When collecting scope dependencies, mark each dependency with `reactive: true | false`. This prepares for later PRs #33326 and #32099 which rewrite scope dependencies into instructions. Note that some reactive objects may have non-reactive properties, but we do not currently track this. Technically, state[0] is reactive and state[1] is not. Currently, both would be marked as reactive. ```js const state = useState(); ```

mofeiZ · 2025-05-21T19:14:43Z

compiler/packages/babel-plugin-react-compiler/src/HIR/ScopeDependencyUtils.ts

+  const builder = new HIRBuilder(env, {
+    entryBlockKind: 'value',
+  });


Reusing HIRBuilder here since writing optional blocks needs the same reserve / enterReserved logic as buildHIR

Ahh this makes sense now, definitely makes it easier to construct the optionals

mofeiZ · 2025-05-21T19:16:30Z

compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts

@@ -86,6 +92,7 @@ export function inferEffectDependencies(fn: HIRFunction): void {
   * reactive(Identifier i) = Union_{reference of i}(reactive(reference))
   */
  const reactiveIds = inferReactiveIdentifiers(fn);
+  const rewriteBlocks: Array<BasicBlock> = [];


We're now inserting new blocks into the function (see rewriteSplices)

mofeiZ · 2025-05-21T19:17:20Z

compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts

              ) {
                // exclude non-reactive hook results, which will never be in a memo block
                continue;
              }

-              const {place, instructions} = writeDependencyToInstructions(
+              const dep = truncateDepAtCurrent(maybeDep);


Moved this check from writeDependencyToInstructions so that IDE highlighting and etc also receive the correct (truncated) dependency

Nice, thank you!

IIUC, the IDE has access to all of the paths but we'll still need to look inside the effect body for property accesses/optional chains that correspond to deps, is that right?

And #32099 would make this easier because it would hoist the deps into temporaries that would be re-used inside the body.

mofeiZ · 2025-05-21T19:21:45Z

compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts

    fn.env.hasInferredEffect = true;
  }
 }

-function writeDependencyToInstructions(


This was moved to ScopeDependencyUtils

josephsavona

Super cool

jbrown215

Wooo! this rocks

jbrown215 · 2025-05-21T18:35:45Z

...ompiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain.js

@@ -4,6 +4,12 @@ import {print} from 'shared-runtime';

 // TODO: take optional chains as dependencies


You can remove the todo now :D

jbrown215 · 2025-05-22T14:43:34Z

...ests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain-complex.expect.md

+import {useEffect} from 'react';
+import {print, shallowCopy} from 'shared-runtime';
+
+// TODO: take optional chains as dependencies


You can drop this now!

jbrown215 · 2025-05-22T14:44:39Z

...src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain-complex.js

+import {useEffect} from 'react';
+import {print, shallowCopy} from 'shared-runtime';
+
+// TODO: take optional chains as dependencies


And this one!

jbrown215 · 2025-05-22T14:48:43Z

compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts

              ) {
                // exclude non-reactive hook results, which will never be in a memo block
                continue;
              }

-              const {place, instructions} = writeDependencyToInstructions(
+              const dep = truncateDepAtCurrent(maybeDep);


Nice, thank you!

IIUC, the IDE has access to all of the paths but we'll still need to look inside the effect body for property accesses/optional chains that correspond to deps, is that right?

And #32099 would make this easier because it would hoist the deps into temporaries that would be re-used inside the body.

When collecting scope dependencies, mark each dependency with `reactive: true | false`. This prepares for later PRs #33326 and #32099 which rewrite scope dependencies into instructions. Note that some reactive objects may have non-reactive properties, but we do not currently track this. Technically, state[0] is reactive and state[1] is not. Currently, both would be marked as reactive. ```js const state = useState(); ```

Inferred effect dependencies now include optional chains. This is a temporary solution while #32099 and its followups are worked on. Ideally, we should model reactive scope dependencies in the IR similarly to `ComputeIR` -- dependencies should be hoisted and all references rewritten to use the hoisted dependencies. `

--- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32286). * #33326 * #33325 * __->__ #32286

When collecting scope dependencies, mark each dependency with `reactive: true | false`. This prepares for later PRs #33326 and #32099 which rewrite scope dependencies into instructions. Note that some reactive objects may have non-reactive properties, but we do not currently track this. Technically, state[0] is reactive and state[1] is not. Currently, both would be marked as reactive. ```js const state = useState(); ``` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33325). * #33326 * __->__ #33325 * #32286

--- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32286). * #33326 * #33325 * __->__ #32286 DiffTrain build for [13f2004](13f2004)

…33326) Inferred effect dependencies now include optional chains. This is a temporary solution while #32099 and its followups are worked on. Ideally, we should model reactive scope dependencies in the IR similarly to `ComputeIR` -- dependencies should be hoisted and all references rewritten to use the hoisted dependencies. ` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33326). * __->__ #33326 * #33325 * #32286 DiffTrain build for [0d07288](0d07288)

This was referenced May 21, 2025

[compiler] Prepare HIRBuilder to be used by later passes #32286

Merged

[compiler] Add reactive flag on scope dependencies #33325

Merged

facebook-github-bot added the CLA Signed label May 21, 2025

mofeiZ force-pushed the pr33326 branch 2 times, most recently from 84347ec to 53cc92d Compare May 21, 2025 18:51

mofeiZ marked this pull request as ready for review May 21, 2025 18:52

mofeiZ force-pushed the pr33326 branch 2 times, most recently from ab7d916 to fe7e4c8 Compare May 21, 2025 19:11

mofeiZ commented May 21, 2025

View reviewed changes

mofeiZ force-pushed the pr33326 branch 2 times, most recently from 0dfaec8 to 1a5a050 Compare May 21, 2025 19:21

mofeiZ commented May 21, 2025

View reviewed changes

josephsavona approved these changes May 21, 2025

View reviewed changes

jbrown215 approved these changes May 22, 2025

View reviewed changes

mofeiZ added 3 commits May 22, 2025 15:44

[compiler] Prepare HIRBuilder to be used by later passes

64b555c

mofeiZ force-pushed the pr33326 branch from 1a5a050 to 0b8b8c6 Compare May 22, 2025 20:10

mofeiZ merged commit 0d07288 into main May 22, 2025
27 of 30 checks passed

vercel-release-bot mentioned this pull request May 26, 2025

Upgrade React from 8ce15b0f-20250522 to c0464aed-20250523 vercel/next.js#79637

Merged

eps1lon mentioned this pull request Jun 7, 2025

Upgrade React from 197d6a04-20250424 to latest vercel/next.js#79477

Draft

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[compiler] Inferred effect dependencies now include optional chains #33326

[compiler] Inferred effect dependencies now include optional chains #33326

Uh oh!

mofeiZ commented May 21, 2025 •

edited

Loading

Uh oh!

mofeiZ May 21, 2025

Uh oh!

josephsavona May 21, 2025

Uh oh!

mofeiZ May 21, 2025

Uh oh!

mofeiZ May 21, 2025

Uh oh!

jbrown215 May 22, 2025

Uh oh!

mofeiZ May 21, 2025

Uh oh!

josephsavona left a comment

Uh oh!

jbrown215 left a comment

Uh oh!

jbrown215 May 21, 2025

Uh oh!

jbrown215 May 22, 2025

Uh oh!

jbrown215 May 22, 2025

Uh oh!

jbrown215 May 22, 2025

Uh oh!

Uh oh!

Uh oh!

		@@ -4,6 +4,12 @@ import {print} from 'shared-runtime';

		// TODO: take optional chains as dependencies

[compiler] Inferred effect dependencies now include optional chains #33326

[compiler] Inferred effect dependencies now include optional chains #33326

Uh oh!

Conversation

mofeiZ commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

`

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josephsavona left a comment

Choose a reason for hiding this comment

Uh oh!

jbrown215 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mofeiZ commented May 21, 2025 •

edited

Loading