Skip to content

Commit

Permalink
feat<Compiler>: consider that the dispatch function from useReducer
Browse files Browse the repository at this point in the history
… is non-reactive (#29705)

Summary
The dispatch function from useReducer is stable, so it is also non-reactive.

the related PR: #29665
the related comment: #29674 (comment)

I am not sure if the location of the new test file is appropriate😅.

How did you test this change?
Added the specific test compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md.
  • Loading branch information
TrickyPi authored Jun 5, 2024
1 parent 3730b40 commit 704aeed
Show file tree
Hide file tree
Showing 10 changed files with 169 additions and 2 deletions.
13 changes: 13 additions & 0 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
BuiltInUseInsertionEffectHookId,
BuiltInUseLayoutEffectHookId,
BuiltInUseOperatorId,
BuiltInUseReducerId,
BuiltInUseRefId,
BuiltInUseStateId,
ShapeRegistry,
Expand Down Expand Up @@ -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, {
Expand Down
13 changes: 13 additions & 0 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1254,6 +1254,11 @@ export enum ValueReason {
*/
State = "state",

/**
* A value returned from `useReducer`
*/
ReducerState = "reducer-state",

/**
* Props of a component or arguments of a hook.
*/
Expand Down Expand Up @@ -1493,6 +1498,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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ function addShape(
export type HookKind =
| "useContext"
| "useState"
| "useReducer"
| "useRef"
| "useEffect"
| "useLayoutEffect"
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 }],
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
Place,
computePostDominatorTree,
getHookKind,
isDispatcherType,
isSetStateType,
isUseOperator,
} from "../HIR";
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
ReactiveFunction,
ReactiveInstruction,
ReactiveScopeBlock,
isDispatcherType,
isSetStateType,
} from "../HIR";
import { eachPatternOperand } from "../HIR/visitors";
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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 |
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { useReducer } from "react";

function Foo() {
let [state, setState] = useReducer({ foo: 1 });
state.foo = 1;
return state;
}
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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,
};

0 comments on commit 704aeed

Please sign in to comment.