@samitouri / QOS-React / commits / 896d1b0027

[dx] Improve error message from InferReferenceEffects

ghstack-source-id: 06265d9676b671a5b02ca05433a219dd219be4f1 Pull Request resolved: https://github.com/facebook/react-forget/pull/2883

Joe Savona committed Apr 22, 2024 at 08:14 UTC 896d1b00273958ddad6cb8b1fafae3cd13cced5a
25 files changed +37 -35
compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts
+13 -11
@@ -541,8 +541,9 @@ class InferenceState {
541 error: {
542 reason,
543 description:
544 - place.identifier.name !== null
545 - ? `Found mutation of ${place.identifier.name}`
544 + place.identifier.name !== null &&
545 + place.identifier.name.kind === "named"
546 + ? `Found mutation of \`${place.identifier.name.value}\``
547 : null,
548 loc: place.loc,
549 suggestions: null,
@@ -575,8 +576,9 @@ class InferenceState {
576 error: {
577 reason,
578 description:
578 - place.identifier.name !== null
579 - ? `Found mutation of ${place.identifier.name}`
579 + place.identifier.name !== null &&
580 + place.identifier.name.kind === "named"
581 + ? `Found mutation of \`${place.identifier.name.value}\``
582 : null,
583 loc: place.loc,
584 suggestions: null,
@@ -1986,18 +1988,18 @@ function areArgumentsImmutableAndNonMutating(
1988
1989 function getWriteErrorReason(abstractValue: AbstractValue): string {
1990 if (abstractValue.reason.has(ValueReason.Global)) {
1989 - return "Writing to a variable defined outside a component or hook is not allowed. Consider using an effect.";
1991 + return "Writing to a variable defined outside a component or hook is not allowed. Consider using an effect";
1992 } else if (abstractValue.reason.has(ValueReason.JsxCaptured)) {
1991 - return "Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX.";
1993 + return "Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX";
1994 } else if (abstractValue.reason.has(ValueReason.Context)) {
1993 - return `Mutating a value returned from 'useContext()', which should not be mutated.`;
1995 + return `Mutating a value returned from 'useContext()', which should not be mutated`;
1996 } else if (abstractValue.reason.has(ValueReason.KnownReturnSignature)) {
1995 - return "Mutating a value returned from a function that should not be mutated.";
1997 + return "Mutating a value returned from a function whose return value should not be mutated";
1998 } else if (abstractValue.reason.has(ValueReason.ReactiveFunctionArgument)) {
1997 - return "Mutating props or hook arguments is not allowed. Consider using a local variable instead.";
1999 + return "Mutating component props or hook arguments is not allowed. Consider using a local variable instead";
2000 } else if (abstractValue.reason.has(ValueReason.State)) {
1999 - return "Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead.";
2001 + return "Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead";
2002 } else {
2001 - return "This mutates a variable that React considers immutable.";
2003 + return "This mutates a variable that React considers immutable";
2004 }
2005 }
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-array-push-frozen.expect.md
+1 -1
@@ -18,7 +18,7 @@ function Component(props) {
18 2 | const x = [];
19 3 | <div>{x}</div>;
20 > 4 | x.push(props.value);
21 - | ^ InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX. (4:4)
21 + | ^ InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX (4:4)
22 5 | return x;
23 6 | }
24 7 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-computed-store-to-frozen-value.expect.md
+1 -1
@@ -19,7 +19,7 @@ function Component(props) {
19 3 | // freeze
20 4 | <div>{x}</div>;
21 > 5 | x[0] = true;
22 - | ^ InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX. (5:5)
22 + | ^ InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX (5:5)
23 6 | return x;
24 7 | }
25 8 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-delete-computed-property-of-frozen-value.expect.md
+1 -1
@@ -19,7 +19,7 @@ function Component(props) {
19 3 | // freeze
20 4 | <div>{x}</div>;
21 > 5 | delete x[y];
22 - | ^ InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX. (5:5)
22 + | ^ InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX (5:5)
23 6 | return x;
24 7 | }
25 8 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-delete-property-of-frozen-value.expect.md
+1 -1
@@ -19,7 +19,7 @@ function Component(props) {
19 3 | // freeze
20 4 | <div>{x}</div>;
21 > 5 | delete x.y;
22 - | ^ InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX. (5:5)
22 + | ^ InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX (5:5)
23 6 | return x;
24 7 | }
25 8 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-function-expression-mutates-immutable-value.expect.md
+1 -1
@@ -21,7 +21,7 @@ function Component(props) {
21 3 | const onChange = (e) => {
22 4 | // INVALID! should use copy-on-write and pass the new value
23 > 5 | x.value = e.target.value;
24 - | ^ InvalidReact: Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead.. Found mutation of [object Object] (5:5)
24 + | ^ 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)
25 6 | setX(x);
26 7 | };
27 8 | return <input value={x.value} onChange={onChange} />;
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-after-aliased-freeze.expect.md
+1 -1
@@ -28,7 +28,7 @@ function Component(props) {
28 11 | // y is MaybeFrozen at this point, since it may alias to x
29 12 | // (which is the above line freezes)
30 > 13 | y.push(props.p2);
31 - | ^ InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX. (13:13)
31 + | ^ InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX (13:13)
32 14 |
33 15 | return <Component x={x} y={y} />;
34 16 | }
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-after-freeze.expect.md
+1 -1
@@ -22,7 +22,7 @@ function Component(props) {
22 5 |
23 6 | // x is Frozen at this point
24 > 7 | x.push(props.p2);
25 - | ^ InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX. (7:7)
25 + | ^ InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX (7:7)
26 8 |
27 9 | return <div>{_}</div>;
28 10 | }
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-context-in-callback.expect.md
+1 -1
@@ -27,7 +27,7 @@ function Component(props) {
27 10 | // independently
28 11 | const onClick = () => {
29 > 12 | FooContext.current = true;
30 - | ^^^^^^^^^^ InvalidReact: Mutating a value returned from 'useContext()', which should not be mutated.. Found mutation of [object Object] (12:12)
30 + | ^^^^^^^^^^ InvalidReact: Mutating a value returned from 'useContext()', which should not be mutated. Found mutation of `FooContext` (12:12)
31 13 | };
32 14 | return <div onClick={onClick} />;
33 15 | }
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-context.expect.md
+1 -1
@@ -17,7 +17,7 @@ function Component(props) {
17 1 | function Component(props) {
18 2 | const context = useContext(FooContext);
19 > 3 | context.value = props.value;
20 - | ^^^^^^^ InvalidReact: Mutating a value returned from 'useContext()', which should not be mutated. (3:3)
20 + | ^^^^^^^ InvalidReact: Mutating a value returned from 'useContext()', which should not be mutated (3:3)
21 4 | return context.value;
22 5 | }
23 6 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-props-in-effect-fixpoint.expect.md
+1 -1
@@ -28,7 +28,7 @@ function Component(props) {
28 8 | let y = x;
29 9 | let mutateProps = () => {
30 > 10 | y.foo = true;
31 - | ^ InvalidReact: This mutates a variable that React considers immutable.. Found mutation of [object Object] (10:10)
31 + | ^ InvalidReact: This mutates a variable that React considers immutable. Found mutation of `y` (10:10)
32 11 | };
33 12 | let mutatePropsIndirect = () => {
34 13 | mutateProps();
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutation-in-closure.expect.md
+1 -1
@@ -19,7 +19,7 @@ function useInvalidMutation(options) {
19 2 | function test() {
20 3 | foo(options.foo); // error should not point on this line
21 > 4 | options.foo = "bar";
22 - | ^^^^^^^ InvalidReact: Mutating props or hook arguments is not allowed. Consider using a local variable instead.. Found mutation of [object Object] (4:4)
22 + | ^^^^^^^ InvalidReact: Mutating component props or hook arguments is not allowed. Consider using a local variable instead. Found mutation of `options` (4:4)
23 5 | }
24 6 | return test;
25 7 | }
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutation-of-possible-props-phi-indirect.expect.md
+1 -1
@@ -22,7 +22,7 @@ function Component(props) {
22 2 | let x = cond ? someGlobal : props.foo;
23 3 | const mutatePhiThatCouldBeProps = () => {
24 > 4 | x.y = true;
25 - | ^ InvalidReact: Writing to a variable defined outside a component or hook is not allowed. Consider using an effect.. Found mutation of [object Object] (4:4)
25 + | ^ InvalidReact: Writing to a variable defined outside a component or hook is not allowed. Consider using an effect. Found mutation of `x` (4:4)
26 5 | };
27 6 | const indirectMutateProps = () => {
28 7 | mutatePhiThatCouldBeProps();
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-prop-mutation-indirect.expect.md
+1 -1
@@ -21,7 +21,7 @@ function Component(props) {
21 1 | function Component(props) {
22 2 | const f = () => {
23 > 3 | props.value = true;
24 - | ^^^^^ InvalidReact: Mutating props or hook arguments is not allowed. Consider using a local variable instead.. Found mutation of [object Object] (3:3)
24 + | ^^^^^ InvalidReact: Mutating component props or hook arguments is not allowed. Consider using a local variable instead. Found mutation of `props` (3:3)
25 4 | };
26 5 | const g = () => {
27 6 | f();
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-property-store-to-frozen-value.expect.md
+1 -1
@@ -19,7 +19,7 @@ function Component(props) {
19 3 | // freeze
20 4 | <div>{x}</div>;
21 > 5 | x.y = true;
22 - | ^ InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX. (5:5)
22 + | ^ InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX (5:5)
23 6 | return x;
24 7 | }
25 8 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-props-mutation-in-effect-indirect.expect.md
+1 -1
@@ -21,7 +21,7 @@ function Component(props) {
21 1 | function Component(props) {
22 2 | const mutateProps = () => {
23 > 3 | props.value = true;
24 - | ^^^^^ InvalidReact: Mutating props or hook arguments is not allowed. Consider using a local variable instead.. Found mutation of [object Object] (3:3)
24 + | ^^^^^ InvalidReact: Mutating component props or hook arguments is not allowed. Consider using a local variable instead. Found mutation of `props` (3:3)
25 4 | };
26 5 | const indirectMutateProps = () => {
27 6 | mutateProps();
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.modify-state-2.expect.md
+1 -1
@@ -20,7 +20,7 @@ function Foo() {
20 4 | const [state, setState] = useState({ foo: { bar: 3 } });
21 5 | const foo = state.foo;
22 > 6 | foo.bar = 1;
23 - | ^^^ InvalidReact: Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead. (6:6)
23 + | ^^^ InvalidReact: Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead (6:6)
24 7 | return state;
25 8 | }
26 9 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.modify-state.expect.md
+1 -1
@@ -19,7 +19,7 @@ function Foo() {
19 3 | function Foo() {
20 4 | let [state, setState] = useState({});
21 > 5 | state.foo = 1;
22 - | ^^^^^ InvalidReact: Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead. (5:5)
22 + | ^^^^^ InvalidReact: Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead (5:5)
23 6 | return state;
24 7 | }
25 8 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.mutate-function-property.expect.md
+1 -1
@@ -18,7 +18,7 @@ export function ViewModeSelector(props) {
18 1 | export function ViewModeSelector(props) {
19 2 | const renderIcon = () => <AcceptIcon />;
20 > 3 | renderIcon.displayName = "AcceptIcon";
21 - | ^^^^^^^^^^ InvalidReact: This mutates a variable that React considers immutable. (3:3)
21 + | ^^^^^^^^^^ InvalidReact: This mutates a variable that React considers immutable (3:3)
22 4 |
23 5 | return <Dropdown checkableIndicator={{ children: renderIcon }} />;
24 6 | }
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.mutate-hook-argument.expect.md
+1 -1
@@ -15,7 +15,7 @@ function useHook(a, b) {
15 ```
16 1 | function useHook(a, b) {
17 > 2 | b.test = 1;
18 - | ^ InvalidReact: Mutating props or hook arguments is not allowed. Consider using a local variable instead. (2:2)
18 + | ^ InvalidReact: Mutating component props or hook arguments is not allowed. Consider using a local variable instead (2:2)
19 3 | a.test = 2;
20 4 | }
21 5 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.mutate-property-from-global.expect.md
+1 -1
@@ -18,7 +18,7 @@ function Foo() {
18 2 |
19 3 | function Foo() {
20 > 4 | delete wat.foo;
21 - | ^^^ InvalidReact: Writing to a variable defined outside a component or hook is not allowed. Consider using an effect. (4:4)
21 + | ^^^ InvalidReact: Writing to a variable defined outside a component or hook is not allowed. Consider using an effect (4:4)
22 5 | return wat;
23 6 | }
24 7 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.mutate-props.expect.md
+1 -1
@@ -15,7 +15,7 @@ function Foo(props) {
15 ```
16 1 | function Foo(props) {
17 > 2 | props.test = 1;
18 - | ^^^^^ InvalidReact: Mutating props or hook arguments is not allowed. Consider using a local variable instead. (2:2)
18 + | ^^^^^ InvalidReact: Mutating component props or hook arguments is not allowed. Consider using a local variable instead (2:2)
19 3 | return null;
20 4 | }
21 5 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.not-useEffect-external-mutate.expect.md
+1 -1
@@ -20,7 +20,7 @@ function Component(props) {
20 3 | function Component(props) {
21 4 | foo(() => {
22 > 5 | x.a = 10;
23 - | ^ InvalidReact: Writing to a variable defined outside a component or hook is not allowed. Consider using an effect. (5:5)
23 + | ^ InvalidReact: Writing to a variable defined outside a component or hook is not allowed. Consider using an effect (5:5)
24 6 | x.a = 20;
25 7 | });
26 8 | }
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.store-property-in-global.expect.md
+1 -1
@@ -18,7 +18,7 @@ function Foo() {
18 2 |
19 3 | function Foo() {
20 > 4 | wat.test = 1;
21 - | ^^^ InvalidReact: Writing to a variable defined outside a component or hook is not allowed. Consider using an effect. (4:4)
21 + | ^^^ InvalidReact: Writing to a variable defined outside a component or hook is not allowed. Consider using an effect (4:4)
22 5 | return wat;
23 6 | }
24 7 |
compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRuleTypescript-test.ts
+1 -1
@@ -52,7 +52,7 @@ const tests: CompilerTestCases = {
52 errors: [
53 {
54 message:
55 - "Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead.",
55 + "Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead",
56 line: 7,
57 },
58 ],