[dx] Update error message for reassigning globals
ghstack-source-id: 48ce4b55a8b4b5a22c8ec29f6787732ab2a67778 Pull Request resolved: https://github.com/facebook/react-forget/pull/2860
Joe Savona committed
Apr 18, 2024 at 07:46 UTC
c84ff16afba0d5b72d0252e0e96be71fcb16848f
6 files changed
+29
-8
compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts
+10
-3
@@ -16,7 +16,7 @@ import {
16
} from "../CompilerError";
17
import { Err, Ok, Result } from "../Utils/Result";
18
import { assertExhaustive, hasNode } from "../Utils/utils";
19
-import { Environment } from "./Environment";
19
+import { Environment, printFunctionType } from "./Environment";
20
import {
21
ArrayExpression,
22
ArrayPattern,
@@ -3273,9 +3273,16 @@ function lowerIdentifierForAssignment(
3273
const identifier = builder.resolveIdentifier(path);
3274
if (identifier == null) {
3275
if (kind === InstructionKind.Reassign) {
3276
- // Trying to reassign a global is not allowed
3276
+ /*
3277
+ * Trying to reassign a global is not allowed
3278
+ * TODO: add support for StoreGlobal or similar, and move this error to run conditionally only for render-phase
3279
+ * functions in InferReferenceEffects
3280
+ */
3281
builder.errors.push({
3278
- reason: `This reassigns a variable which was not defined inside of the component. Components should be pure and side-effect free. If this variable is used in rendering, use useState instead. (https://react.dev/learn/keeping-components-pure)`,
3282
+ reason: `Unexpected reassignment of a variable which was defined outside of the ${printFunctionType(
3283
+ builder.environment.fnType
3284
+ )}`,
3285
+ description: `Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render)`,
3286
severity: ErrorSeverity.InvalidReact,
3287
loc: path.parentPath.node.loc ?? null,
3288
suggestions: null,
compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts
+14
@@ -405,6 +405,20 @@ export type PartialEnvironmentConfig = Partial<EnvironmentConfig>;
405
406
export type ReactFunctionType = "Component" | "Hook" | "Other";
407
408
+export function printFunctionType(type: ReactFunctionType): string {
409
+ switch (type) {
410
+ case "Component": {
411
+ return "component";
412
+ }
413
+ case "Hook": {
414
+ return "hook";
415
+ }
416
+ default: {
417
+ return "function";
418
+ }
419
+ }
420
+}
421
+
422
export class Environment {
423
#globals: GlobalRegistry;
424
#shapes: ShapeRegistry;
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-destructure-assignment-to-global.expect.md
+1
-1
@@ -15,7 +15,7 @@ function useFoo(props) {
15
```
16
1 | function useFoo(props) {
17
> 2 | [x] = props;
18
- | ^^^ InvalidReact: This reassigns a variable which was not defined inside of the component. Components should be pure and side-effect free. If this variable is used in rendering, use useState instead. (https://react.dev/learn/keeping-components-pure) (2:2)
18
+ | ^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the function. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (2:2)
19
3 | return { x };
20
4 | }
21
5 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-destructure-to-local-global-variables.expect.md
+1
-1
@@ -18,7 +18,7 @@ function Component(props) {
18
1 | function Component(props) {
19
2 | let a;
20
> 3 | [a, b] = props.value;
21
- | ^^^^^^ InvalidReact: This reassigns a variable which was not defined inside of the component. Components should be pure and side-effect free. If this variable is used in rendering, use useState instead. (https://react.dev/learn/keeping-components-pure) (3:3)
21
+ | ^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the function. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (3:3)
22
4 |
23
5 | return [a, b];
24
6 | }
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.mutate-global-increment-op-invalid-react.expect.md
+1
-1
@@ -18,7 +18,7 @@ function NoHooks() {
18
2 |
19
3 | function NoHooks() {
20
> 4 | renderCount++;
21
- | ^^^^^^^^^^^^^ InvalidReact: This reassigns a variable which was not defined inside of the component. Components should be pure and side-effect free. If this variable is used in rendering, use useState instead. (https://react.dev/learn/keeping-components-pure) (4:4)
21
+ | ^^^^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (4:4)
22
5 | return <div />;
23
6 | }
24
7 |
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.reassignment-to-global.expect.md
+2
-2
@@ -17,9 +17,9 @@ function Component() {
17
1 | function Component() {
18
2 | // Cannot assign to globals
19
> 3 | someUnknownGlobal = true;
20
- | ^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: This reassigns a variable which was not defined inside of the component. Components should be pure and side-effect free. If this variable is used in rendering, use useState instead. (https://react.dev/learn/keeping-components-pure) (3:3)
20
+ | ^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the function. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (3:3)
21
22
-InvalidReact: This reassigns a variable which was not defined inside of the component. Components should be pure and side-effect free. If this variable is used in rendering, use useState instead. (https://react.dev/learn/keeping-components-pure) (4:4)
22
+InvalidReact: Unexpected reassignment of a variable which was defined outside of the function. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (4:4)
23
4 | moduleLocal = true;
24
5 | }
25
6 |