From 4b236a5ae6082e025b94b9eccc0a76faa3078c7a Mon Sep 17 00:00:00 2001 From: XiaoPi <530257315@qq.com> Date: Sat, 1 Jun 2024 21:22:29 +0800 Subject: [PATCH 1/4] feat: Consider dispatch from useReducer is non-reactive --- .../src/HIR/Globals.ts | 13 +++++ .../src/HIR/HIR.ts | 12 +++- .../src/HIR/ObjectShape.ts | 22 +++++++ .../src/Inference/InferReactivePlaces.ts | 3 +- .../src/Inference/InferReferenceEffects.ts | 2 +- .../PruneNonReactiveDependencies.ts | 3 +- ...pression-mutates-immutable-value.expect.md | 2 +- .../compiler/error.modify-state-2.expect.md | 2 +- .../compiler/error.modify-state.expect.md | 2 +- ...urned-dispatcher-is-non-reactive.expect.md | 57 +++++++++++++++++++ ...cer-returned-dispatcher-is-non-reactive.js | 17 ++++++ .../ReactCompilerRuleTypescript-test.ts | 2 +- 12 files changed, 128 insertions(+), 9 deletions(-) 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..020d45a9525c7 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.State, + }), + ], [ "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..282cecd5d83cf 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -1250,7 +1250,7 @@ export enum ValueReason { Context = "context", /** - * A value returned from `useState` + * A value returned from `useState` or `useReducer` */ State = "state", @@ -1490,7 +1490,15 @@ export function isUseStateType(id: Identifier): boolean { } export function isSetStateType(id: Identifier): boolean { - return id.type.kind === "Function" && id.type.shapeId === "BuiltInSetState"; + 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 { 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..164e1725e1d26 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,7 @@ 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..cb271344c9011 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts @@ -2116,7 +2116,7 @@ function getWriteErrorReason(abstractValue: AbstractValue): string { } else if (abstractValue.reason.has(ValueReason.ReactiveFunctionArgument)) { 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"; + return "Mutating a value returned from 'useState() or useReducer()', which should not be mutated. Use the setter or dispatcher 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..3d823548cf2bc 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,7 @@ 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.invalid-function-expression-mutates-immutable-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-function-expression-mutates-immutable-value.expect.md index 933495c5a4f9b..4bac923a43663 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-function-expression-mutates-immutable-value.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-function-expression-mutates-immutable-value.expect.md @@ -21,7 +21,7 @@ function Component(props) { 3 | const onChange = (e) => { 4 | // INVALID! should use copy-on-write and pass the new value > 5 | x.value = e.target.value; - | ^ InvalidReact: Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead. Found mutation of `x` (5:5) + | ^ InvalidReact: Mutating a value returned from 'useState() or useReducer()', which should not be mutated. Use the setter or dispatcher function to update instead. Found mutation of `x` (5:5) 6 | setX(x); 7 | }; 8 | return ; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-state-2.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-state-2.expect.md index 86faecc63222d..6ab8e0e49ac38 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-state-2.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-state-2.expect.md @@ -20,7 +20,7 @@ function Foo() { 4 | const [state, setState] = useState({ foo: { bar: 3 } }); 5 | const foo = state.foo; > 6 | foo.bar = 1; - | ^^^ InvalidReact: Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead (6:6) + | ^^^ InvalidReact: Mutating a value returned from 'useState() or useReducer()', which should not be mutated. Use the setter or dispatcher function to update instead (6:6) 7 | return state; 8 | } 9 | diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-state.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-state.expect.md index 17302a8dae89c..9f31100838507 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-state.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-state.expect.md @@ -19,7 +19,7 @@ function Foo() { 3 | function Foo() { 4 | let [state, setState] = useState({}); > 5 | state.foo = 1; - | ^^^^^ InvalidReact: Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead (5:5) + | ^^^^^ InvalidReact: Mutating a value returned from 'useState() or useReducer()', which should not be mutated. Use the setter or dispatcher function to update instead (5:5) 6 | return state; 7 | } 8 | 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..84ef54c12e9fb --- /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)
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..04d4c5d991c09 --- /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, +}; diff --git a/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRuleTypescript-test.ts b/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRuleTypescript-test.ts index 9c44f62ef81ef..388df8371448b 100644 --- a/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRuleTypescript-test.ts +++ b/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRuleTypescript-test.ts @@ -62,7 +62,7 @@ const tests: CompilerTestCases = { errors: [ { message: - "Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead", + "Mutating a value returned from 'useState() or useReducer()', which should not be mutated. Use the setter or dispatcher function to update instead", line: 7, }, ], From 510d05a7c02ea6948788a556063ad568f07ce0cc Mon Sep 17 00:00:00 2001 From: XiaoPi <530257315@qq.com> Date: Sat, 1 Jun 2024 23:02:37 +0800 Subject: [PATCH 2/4] prettier --- compiler/apps/playground/components/TabbedWindow.tsx | 8 ++++++-- .../packages/babel-plugin-react-compiler/src/HIR/HIR.ts | 2 +- .../src/Inference/InferReactivePlaces.ts | 5 ++++- .../src/ReactiveScopes/PruneNonReactiveDependencies.ts | 5 ++++- .../useReducer-returned-dispatcher-is-non-reactive.js | 2 +- 5 files changed, 16 insertions(+), 6 deletions(-) diff --git a/compiler/apps/playground/components/TabbedWindow.tsx b/compiler/apps/playground/components/TabbedWindow.tsx index 6d7ed3517bf77..b77ecb0ecdbe1 100644 --- a/compiler/apps/playground/components/TabbedWindow.tsx +++ b/compiler/apps/playground/components/TabbedWindow.tsx @@ -78,7 +78,9 @@ function TabbedWindowItem({ title="Minimize tab" aria-label="Minimize tab" onClick={toggleTabs} - className={`p-4 duration-150 ease-in border-b cursor-pointer border-grey-200 ${hasChanged ? "font-bold" : "font-light"} text-secondary hover:text-link`} + className={`p-4 duration-150 ease-in border-b cursor-pointer border-grey-200 ${ + hasChanged ? "font-bold" : "font-light" + } text-secondary hover:text-link`} > - {name} @@ -91,7 +93,9 @@ function TabbedWindowItem({ aria-label={`Expand compiler tab: ${name}`} style={{ transform: "rotate(90deg) translate(-50%)" }} onClick={toggleTabs} - className={`flex-grow-0 w-5 transition-colors duration-150 ease-in ${hasChanged ? "font-bold" : "font-light"} text-secondary hover:text-link`} + className={`flex-grow-0 w-5 transition-colors duration-150 ease-in ${ + hasChanged ? "font-bold" : "font-light" + } text-secondary hover:text-link`} > {name} 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 282cecd5d83cf..01025f9354222 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -1490,7 +1490,7 @@ export function isUseStateType(id: Identifier): boolean { } export function isSetStateType(id: Identifier): boolean { - return id.type.kind === "Function" && (id.type.shapeId === "BuiltInSetState"); + return id.type.kind === "Function" && id.type.shapeId === "BuiltInSetState"; } export function isUseReducerType(id: Identifier): boolean { 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 164e1725e1d26..e6a7bb49ce132 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts @@ -220,7 +220,10 @@ export function inferReactivePlaces(fn: HIRFunction): void { if (hasReactiveInput) { for (const lvalue of eachInstructionLValue(instruction)) { - if (isSetStateType(lvalue.identifier)||isDispatcherType(lvalue.identifier)) { + if ( + isSetStateType(lvalue.identifier) || + isDispatcherType(lvalue.identifier) + ) { continue; } reactiveIdentifiers.markReactive(lvalue); 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 3d823548cf2bc..aef5d50ee3a06 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts @@ -57,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) || isDispatcherType(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/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 index 04d4c5d991c09..c1dec4e5a7f00 100644 --- 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 @@ -5,7 +5,7 @@ function f() { const onClick = () => { dispatch(); - } + }; return
; } From 723daa962c15d8700703d551be250a0f2d42e4a8 Mon Sep 17 00:00:00 2001 From: XiaoPi <530257315@qq.com> Date: Sun, 2 Jun 2024 19:38:04 +0800 Subject: [PATCH 3/4] Introduce a new ValueReason enum field ReducerState --- .../src/HIR/Globals.ts | 2 +- .../src/HIR/HIR.ts | 7 ++++- .../src/Inference/InferReferenceEffects.ts | 6 ++-- ...pression-mutates-immutable-value.expect.md | 2 +- .../compiler/error.modify-state-2.expect.md | 2 +- .../compiler/error.modify-state.expect.md | 2 +- .../error.modify-useReducer-state.expect.md | 28 +++++++++++++++++++ .../compiler/error.modify-useReducer-state.js | 7 +++++ .../ReactCompilerRuleTypescript-test.ts | 2 +- 9 files changed, 50 insertions(+), 8 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 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 020d45a9525c7..041d2fbf00911 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts @@ -275,7 +275,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ calleeEffect: Effect.Read, hookKind: "useReducer", returnValueKind: ValueKind.Frozen, - returnValueReason: ValueReason.State, + returnValueReason: ValueReason.ReducerState, }), ], [ 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 01025f9354222..afa0799b40d26 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -1250,10 +1250,15 @@ export enum ValueReason { Context = "context", /** - * A value returned from `useState` or `useReducer` + * A value returned from `useState` */ State = "state", + /** + * A value returned from `useReducer` + */ + ReducerState = "reducer-state", + /** * Props of a component or arguments of a hook. */ 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 cb271344c9011..84ad08627398f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts @@ -2116,8 +2116,10 @@ function getWriteErrorReason(abstractValue: AbstractValue): string { } else if (abstractValue.reason.has(ValueReason.ReactiveFunctionArgument)) { 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() or useReducer()', which should not be mutated. Use the setter or dispatcher function to update instead"; - } else { + 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"; + } { return "This mutates a variable that React considers immutable"; } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-function-expression-mutates-immutable-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-function-expression-mutates-immutable-value.expect.md index 4bac923a43663..933495c5a4f9b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-function-expression-mutates-immutable-value.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-function-expression-mutates-immutable-value.expect.md @@ -21,7 +21,7 @@ function Component(props) { 3 | const onChange = (e) => { 4 | // INVALID! should use copy-on-write and pass the new value > 5 | x.value = e.target.value; - | ^ InvalidReact: Mutating a value returned from 'useState() or useReducer()', which should not be mutated. Use the setter or dispatcher function to update instead. Found mutation of `x` (5:5) + | ^ InvalidReact: Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead. Found mutation of `x` (5:5) 6 | setX(x); 7 | }; 8 | return ; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-state-2.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-state-2.expect.md index 6ab8e0e49ac38..86faecc63222d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-state-2.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-state-2.expect.md @@ -20,7 +20,7 @@ function Foo() { 4 | const [state, setState] = useState({ foo: { bar: 3 } }); 5 | const foo = state.foo; > 6 | foo.bar = 1; - | ^^^ InvalidReact: Mutating a value returned from 'useState() or useReducer()', which should not be mutated. Use the setter or dispatcher function to update instead (6:6) + | ^^^ InvalidReact: Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead (6:6) 7 | return state; 8 | } 9 | diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-state.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-state.expect.md index 9f31100838507..17302a8dae89c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-state.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-state.expect.md @@ -19,7 +19,7 @@ function Foo() { 3 | function Foo() { 4 | let [state, setState] = useState({}); > 5 | state.foo = 1; - | ^^^^^ InvalidReact: Mutating a value returned from 'useState() or useReducer()', which should not be mutated. Use the setter or dispatcher function to update instead (5:5) + | ^^^^^ InvalidReact: Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead (5:5) 6 | return state; 7 | } 8 | 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..81800830b2c07 --- /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..c70a36b8e6d95 --- /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/eslint-plugin-react-compiler/__tests__/ReactCompilerRuleTypescript-test.ts b/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRuleTypescript-test.ts index 388df8371448b..9c44f62ef81ef 100644 --- a/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRuleTypescript-test.ts +++ b/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRuleTypescript-test.ts @@ -62,7 +62,7 @@ const tests: CompilerTestCases = { errors: [ { message: - "Mutating a value returned from 'useState() or useReducer()', which should not be mutated. Use the setter or dispatcher function to update instead", + "Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead", line: 7, }, ], From 8353f1f53a36170c2e316919e36152a94030baea Mon Sep 17 00:00:00 2001 From: XiaoPi <530257315@qq.com> Date: Tue, 4 Jun 2024 09:52:33 +0800 Subject: [PATCH 4/4] prettier --- compiler/apps/playground/components/TabbedWindow.tsx | 8 ++------ .../src/Inference/InferReferenceEffects.ts | 4 ++-- .../compiler/error.modify-useReducer-state.expect.md | 4 ++-- .../fixtures/compiler/error.modify-useReducer-state.js | 2 +- ...eReducer-returned-dispatcher-is-non-reactive.expect.md | 6 +++--- 5 files changed, 10 insertions(+), 14 deletions(-) diff --git a/compiler/apps/playground/components/TabbedWindow.tsx b/compiler/apps/playground/components/TabbedWindow.tsx index b77ecb0ecdbe1..6d7ed3517bf77 100644 --- a/compiler/apps/playground/components/TabbedWindow.tsx +++ b/compiler/apps/playground/components/TabbedWindow.tsx @@ -78,9 +78,7 @@ function TabbedWindowItem({ title="Minimize tab" aria-label="Minimize tab" onClick={toggleTabs} - className={`p-4 duration-150 ease-in border-b cursor-pointer border-grey-200 ${ - hasChanged ? "font-bold" : "font-light" - } text-secondary hover:text-link`} + className={`p-4 duration-150 ease-in border-b cursor-pointer border-grey-200 ${hasChanged ? "font-bold" : "font-light"} text-secondary hover:text-link`} > - {name} @@ -93,9 +91,7 @@ function TabbedWindowItem({ aria-label={`Expand compiler tab: ${name}`} style={{ transform: "rotate(90deg) translate(-50%)" }} onClick={toggleTabs} - className={`flex-grow-0 w-5 transition-colors duration-150 ease-in ${ - hasChanged ? "font-bold" : "font-light" - } text-secondary hover:text-link`} + className={`flex-grow-0 w-5 transition-colors duration-150 ease-in ${hasChanged ? "font-bold" : "font-light"} text-secondary hover:text-link`} > {name} 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 84ad08627398f..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,9 +2117,9 @@ 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)){ + } 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/__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 index 81800830b2c07..22bdff08d8731 100644 --- 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 @@ -5,7 +5,7 @@ import { useReducer } from "react"; function Foo() { - let [state, setState] = useReducer({foo:1}); + let [state, setState] = useReducer({ foo: 1 }); state.foo = 1; return state; } @@ -17,7 +17,7 @@ function Foo() { ``` 3 | function Foo() { - 4 | let [state, setState] = useReducer({foo:1}); + 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; 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 index c70a36b8e6d95..42a04fc8da3d2 100644 --- 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 @@ -1,7 +1,7 @@ import { useReducer } from "react"; function Foo() { - let [state, setState] = useReducer({foo:1}); + 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 index 84ef54c12e9fb..32c0836647cbf 100644 --- 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 @@ -9,7 +9,7 @@ function f() { const onClick = () => { dispatch(); - } + }; return
; } @@ -52,6 +52,6 @@ export const FIXTURE_ENTRYPOINT = { }; ``` - + ### Eval output -(kind: ok)
+(kind: ok)
\ No newline at end of file