From 704aeed022f4277cd5604bf6d76199a6cfe4707f Mon Sep 17 00:00:00 2001 From: XiaoPi <530257315@qq.com> Date: Thu, 6 Jun 2024 07:51:09 +0800 Subject: [PATCH] feat: consider that the dispatch function from `useReducer` is non-reactive (#29705) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../src/HIR/Globals.ts | 13 +++++ .../src/HIR/HIR.ts | 13 +++++ .../src/HIR/ObjectShape.ts | 22 +++++++ .../src/Inference/InferReactivePlaces.ts | 6 +- .../src/Inference/InferReferenceEffects.ts | 2 + .../PruneNonReactiveDependencies.ts | 6 +- .../error.modify-useReducer-state.expect.md | 28 +++++++++ .../compiler/error.modify-useReducer-state.js | 7 +++ ...urned-dispatcher-is-non-reactive.expect.md | 57 +++++++++++++++++++ ...cer-returned-dispatcher-is-non-reactive.js | 17 ++++++ 10 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-useReducer-state.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-useReducer-state.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts index 931d315d308e0..041d2fbf00911 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts +++ b/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, { diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index f9dfea52f363e..afa0799b40d26 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -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. */ @@ -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" diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts index fd04bf43c2950..8997ad086f5a2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts +++ b/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 }], ]); diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts index ad2f666ac16d1..e6a7bb49ce132 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts +++ b/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); diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts index 520684c026bda..387dafb6e5a1f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts +++ b/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"; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts index 0c82cefc59f06..aef5d50ee3a06 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts +++ b/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 { 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); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-useReducer-state.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-useReducer-state.expect.md new file mode 100644 index 0000000000000..22bdff08d8731 --- /dev/null +++ b/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 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-useReducer-state.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-useReducer-state.js new file mode 100644 index 0000000000000..42a04fc8da3d2 --- /dev/null +++ b/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; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md new file mode 100644 index 0000000000000..32c0836647cbf --- /dev/null +++ b/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
; +} + +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 =
; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: f, + params: [], + isComponent: true, +}; + +``` + +### Eval output +(kind: ok)
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.js new file mode 100644 index 0000000000000..c1dec4e5a7f00 --- /dev/null +++ b/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
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: f, + params: [], + isComponent: true, +};