Update on "compiler: Represent pruned scopes instead of inlining"

There are a few places where we want to check whether a value actually got memoized, and we currently have to infer this based on values that "should" have a scope and whether a corresponding scope actually exists. This PR adds a new ReactiveStatement variant to model a reactive scope block that was pruned for some reason, and updates all the passes that prune scopes to instead produce this new variant.

[ghstack-poisoned]

.eslintrc.js

@@ -486,6 +486,7 @@ module.exports = {
     $ReadOnlyArray: 'readonly',
     $ArrayBufferView: 'readonly',
     $Shape: 'readonly',
+    CallSite: 'readonly',
     ConsoleTask: 'readonly', // TOOD: Figure out what the official name of this will be.
     ReturnType: 'readonly',
     AnimationFrameID: 'readonly',

compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts

@@ -30,13 +30,16 @@ export default function BabelPluginReactCompiler(
        */
       Program(prog, pass): void {
         let opts = parsePluginOptions(pass.opts);
+        const isDev =
+          (typeof __DEV__ !== "undefined" && __DEV__ === true) ||
+          process.env["NODE_ENV"] === "development";
         if (
           opts.enableReanimatedCheck === true &&
           pipelineUsesReanimatedPlugin(pass.file.opts.plugins)
         ) {
           opts = injectReanimatedFlag(opts);
         }
-        if (process.env["NODE_ENV"] === "development") {
+        if (isDev) {
           opts = {
             ...opts,
             environment: {

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

@@ -13,6 +13,7 @@ import {
   BuiltInUseInsertionEffectHookId,
   BuiltInUseLayoutEffectHookId,
   BuiltInUseOperatorId,
+  BuiltInUseReducerId,
   BuiltInUseRefId,
   BuiltInUseStateId,
   ShapeRegistry,
@@ -265,6 +266,18 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
       returnValueReason: ValueReason.State,
     }),
   ],
+  [
+    "useReducer",
+    addHook(DEFAULT_SHAPES, {
+      positionalParams: [],
+      restParam: Effect.Freeze,
+      returnType: { kind: "Object", shapeId: BuiltInUseReducerId },
+      calleeEffect: Effect.Read,
+      hookKind: "useReducer",
+      returnValueKind: ValueKind.Frozen,
+      returnValueReason: ValueReason.ReducerState,
+    }),
+  ],
   [
     "useRef",
     addHook(DEFAULT_SHAPES, {

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

@@ -1261,6 +1261,11 @@ export enum ValueReason {
    */
   State = "state",
 
+  /**
+   * A value returned from `useReducer`
+   */
+  ReducerState = "reducer-state",
+
   /**
    * Props of a component or arguments of a hook.
    */
@@ -1500,6 +1505,14 @@ export function isSetStateType(id: Identifier): boolean {
   return id.type.kind === "Function" && id.type.shapeId === "BuiltInSetState";
 }
 
+export function isUseReducerType(id: Identifier): boolean {
+  return id.type.kind === "Function" && id.type.shapeId === "BuiltInUseReducer";
+}
+
+export function isDispatcherType(id: Identifier): boolean {
+  return id.type.kind === "Function" && id.type.shapeId === "BuiltInDispatch";
+}
+
 export function isUseEffectHookType(id: Identifier): boolean {
   return (
     id.type.kind === "Function" && id.type.shapeId === "BuiltInUseEffectHook"

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

@@ -118,6 +118,7 @@ function addShape(
 export type HookKind =
   | "useContext"
   | "useState"
+  | "useReducer"
   | "useRef"
   | "useEffect"
   | "useLayoutEffect"
@@ -200,6 +201,8 @@ export const BuiltInUseEffectHookId = "BuiltInUseEffectHook";
 export const BuiltInUseLayoutEffectHookId = "BuiltInUseLayoutEffectHook";
 export const BuiltInUseInsertionEffectHookId = "BuiltInUseInsertionEffectHook";
 export const BuiltInUseOperatorId = "BuiltInUseOperator";
+export const BuiltInUseReducerId = "BuiltInUseReducer";
+export const BuiltInDispatchId = "BuiltInDispatch";
 
 // ShapeRegistry with default definitions for built-ins.
 export const BUILTIN_SHAPES: ShapeRegistry = new Map();
@@ -387,6 +390,25 @@ addObject(BUILTIN_SHAPES, BuiltInUseStateId, [
   ],
 ]);
 
+addObject(BUILTIN_SHAPES, BuiltInUseReducerId, [
+  ["0", { kind: "Poly" }],
+  [
+    "1",
+    addFunction(
+      BUILTIN_SHAPES,
+      [],
+      {
+        positionalParams: [],
+        restParam: Effect.Freeze,
+        returnType: PRIMITIVE_TYPE,
+        calleeEffect: Effect.Read,
+        returnValueKind: ValueKind.Primitive,
+      },
+      BuiltInDispatchId
+    ),
+  ],
+]);
+
 addObject(BUILTIN_SHAPES, BuiltInUseRefId, [
   ["current", { kind: "Object", shapeId: BuiltInRefValueId }],
 ]);

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

@@ -15,6 +15,7 @@ import {
   Place,
   computePostDominatorTree,
   getHookKind,
+  isDispatcherType,
   isSetStateType,
   isUseOperator,
 } from "../HIR";
@@ -219,7 +220,10 @@ export function inferReactivePlaces(fn: HIRFunction): void {
 
         if (hasReactiveInput) {
           for (const lvalue of eachInstructionLValue(instruction)) {
-            if (isSetStateType(lvalue.identifier)) {
+            if (
+              isSetStateType(lvalue.identifier) ||
+              isDispatcherType(lvalue.identifier)
+            ) {
               continue;
             }
             reactiveIdentifiers.markReactive(lvalue);

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

@@ -2117,6 +2117,8 @@ function getWriteErrorReason(abstractValue: AbstractValue): string {
     return "Mutating component props or hook arguments is not allowed. Consider using a local variable instead";
   } else if (abstractValue.reason.has(ValueReason.State)) {
     return "Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead";
+  } else if (abstractValue.reason.has(ValueReason.ReducerState)) {
+    return "Mutating a value returned from 'useReducer()', which should not be mutated. Use the dispatch function to update instead";
   } else {
     return "This mutates a variable that React considers immutable";
   }

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts

@@ -10,6 +10,7 @@ import {
   ReactiveFunction,
   ReactiveInstruction,
   ReactiveScopeBlock,
+  isDispatcherType,
   isSetStateType,
 } from "../HIR";
 import { eachPatternOperand } from "../HIR/visitors";
@@ -56,7 +57,10 @@ class Visitor extends ReactiveFunctionVisitor<ReactiveIdentifiers> {
       case "Destructure": {
         if (state.has(value.value.identifier.id)) {
           for (const lvalue of eachPatternOperand(value.lvalue.pattern)) {
-            if (isSetStateType(lvalue.identifier)) {
+            if (
+              isSetStateType(lvalue.identifier) ||
+              isDispatcherType(lvalue.identifier)
+            ) {
               continue;
             }
             state.add(lvalue.identifier.id);

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-useReducer-state.expect.md

@@ -0,0 +1,28 @@
+
+## Input
+
+```javascript
+import { useReducer } from "react";
+
+function Foo() {
+  let [state, setState] = useReducer({ foo: 1 });
+  state.foo = 1;
+  return state;
+}
+
+```
+
+
+## Error
+
+```
+  3 | function Foo() {
+  4 |   let [state, setState] = useReducer({ foo: 1 });
+> 5 |   state.foo = 1;
+    |   ^^^^^ InvalidReact: Mutating a value returned from 'useReducer()', which should not be mutated. Use the dispatch function to update instead (5:5)
+  6 |   return state;
+  7 | }
+  8 |
+```
+          
+      

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-useReducer-state.js

@@ -0,0 +1,7 @@
+import { useReducer } from "react";
+
+function Foo() {
+  let [state, setState] = useReducer({ foo: 1 });
+  state.foo = 1;
+  return state;
+}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md

@@ -0,0 +1,57 @@
+
+## Input
+
+```javascript
+import { useReducer } from "react";
+
+function f() {
+  const [state, dispatch] = useReducer();
+
+  const onClick = () => {
+    dispatch();
+  };
+
+  return <div onClick={onClick} />;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: f,
+  params: [],
+  isComponent: true,
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+import { useReducer } from "react";
+
+function f() {
+  const $ = _c(1);
+  const [state, dispatch] = useReducer();
+  let t0;
+  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+    const onClick = () => {
+      dispatch();
+    };
+
+    t0 = <div onClick={onClick} />;
+    $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  return t0;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: f,
+  params: [],
+  isComponent: true,
+};
+
+```
+      
+### Eval output
+(kind: ok) <div></div>

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.js

@@ -0,0 +1,17 @@
+import { useReducer } from "react";
+
+function f() {
+  const [state, dispatch] = useReducer();
+
+  const onClick = () => {
+    dispatch();
+  };
+
+  return <div onClick={onClick} />;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: f,
+  params: [],
+  isComponent: true,
+};

compiler/packages/snap/src/runner-watch.ts

@@ -189,14 +189,15 @@ function subscribeKeyEvents(
   state: RunnerState,
   onChange: (state: RunnerState) => void
 ) {
-  process.stdin.on("keypress", (str, key) => {
+  process.stdin.on("keypress", async (str, key) => {
     if (key.name === "u") {
       // u => update fixtures
       state.mode.action = RunnerAction.Update;
     } else if (key.name === "q") {
       process.exit(0);
     } else if (key.name === "f") {
       state.mode.filter = !state.mode.filter;
+      state.filter = state.mode.filter ? await readTestFilter() : null;
       state.mode.action = RunnerAction.Test;
     } else {
       // any other key re-runs tests

fixtures/flight/server/region.js

@@ -187,7 +187,11 @@ if (process.env.NODE_ENV === 'development') {
       res.set('Content-type', 'application/json');
       let requestedFilePath = req.query.name;
 
+      let isCompiledOutput = false;
       if (requestedFilePath.startsWith('file://')) {
+        // We assume that if it was prefixed with file:// it's referring to the compiled output
+        // and if it's a direct file path we assume it's source mapped back to original format.
+        isCompiledOutput = true;
         requestedFilePath = requestedFilePath.slice(7);
       }
 
@@ -204,11 +208,11 @@ if (process.env.NODE_ENV === 'development') {
       let map;
       // There are two ways to return a source map depending on what we observe in error.stack.
       // A real app will have a similar choice to make for which strategy to pick.
-      if (!sourceMap || Error.prepareStackTrace === undefined) {
-        // When --enable-source-maps is enabled, the error.stack that we use to track
-        // stacks will have had the source map already applied so it's pointing to the
-        // original source. We return a blank source map that just maps everything to
-        // the original source in this case.
+      if (!sourceMap || !isCompiledOutput) {
+        // If a file doesn't have a source map, such as this file, then we generate a blank
+        // source map that just contains the original content and segments pointing to the
+        // original lines.
+        // Similarly
         const sourceContent = await readFile(requestedFilePath, 'utf8');
         const lines = sourceContent.split('\n').length;
         map = {
@@ -222,13 +226,11 @@ if (process.env.NODE_ENV === 'development') {
           sourceRoot: '',
         };
       } else {
-        // If something has overridden prepareStackTrace it is likely not getting the
-        // natively applied source mapping to error.stack and so the line will point to
-        // the compiled output similar to how a browser works.
-        // E.g. ironically this can happen with the source-map-support library that is
-        // auto-invoked by @babel/register if external source maps are generated.
-        // In this case we just use the source map that the native source mapping would
-        // have used.
+        // We always set prepareStackTrace before reading the stack so that we get the stack
+        // without source maps applied. Therefore we have to use the original source map.
+        // If something read .stack before we did, we might observe the line/column after
+        // source mapping back to the original file. We use the isCompiledOutput check above
+        // in that case.
         map = sourceMap.payload;
       }
       res.write(JSON.stringify(map));

packages/react-devtools-core/src/standalone.js

@@ -279,7 +279,6 @@ function initialize(socket: WebSocket) {
   // $FlowFixMe[incompatible-call] found when upgrading Flow
   store = new Store(bridge, {
     checkBridgeProtocolCompatibility: true,
-    supportsNativeInspection: true,
     supportsTraceUpdates: true,
   });
 

packages/react-devtools-extensions/src/main/index.js

@@ -97,6 +97,7 @@ function createBridgeAndStore() {
     // At this time, the timeline can only parse Chrome performance profiles.
     supportsTimeline: __IS_CHROME__,
     supportsTraceUpdates: true,
+    supportsNativeInspection: true,
   });
 
   if (!isProfiling) {

packages/react-devtools-fusebox/src/frontend.js

@@ -37,7 +37,6 @@ export function createStore(bridge: FrontendBridge, config?: Config): Store {
   return new Store(bridge, {
     checkBridgeProtocolCompatibility: true,
     supportsTraceUpdates: true,
-    supportsNativeInspection: true,
     ...config,
   });
 }

packages/react-devtools-inline/src/frontend.js

@@ -23,7 +23,6 @@ export function createStore(bridge: FrontendBridge, config?: Config): Store {
     checkBridgeProtocolCompatibility: true,
     supportsTraceUpdates: true,
     supportsTimeline: true,
-    supportsNativeInspection: true,
     ...config,
   });
 }