Skip to content

Commit 62fc263

Browse files
committed
[compiler] Only run validations with env.logErrors on outputMode: 'lint'
Summary: These validations are not essential for compilation, with this we only run that logic when outputMode is 'lint' Test Plan: Run tests
1 parent 627b583 commit 62fc263

File tree

81 files changed

+479
-1163
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

81 files changed

+479
-1163
lines changed

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -279,17 +279,20 @@ function runWithEnvironment(
279279
validateNoSetStateInRender(hir).unwrap();
280280
}
281281

282-
if (env.config.validateNoDerivedComputationsInEffects_exp) {
282+
if (
283+
env.config.validateNoDerivedComputationsInEffects_exp &&
284+
env.outputMode === 'lint'
285+
) {
283286
env.logErrors(validateNoDerivedComputationsInEffects_exp(hir));
284287
} else if (env.config.validateNoDerivedComputationsInEffects) {
285288
validateNoDerivedComputationsInEffects(hir);
286289
}
287290

288-
if (env.config.validateNoSetStateInEffects) {
291+
if (env.config.validateNoSetStateInEffects && env.outputMode === 'lint') {
289292
env.logErrors(validateNoSetStateInEffects(hir, env));
290293
}
291294

292-
if (env.config.validateNoJSXInTryStatements) {
295+
if (env.config.validateNoJSXInTryStatements && env.outputMode === 'lint') {
293296
env.logErrors(validateNoJSXInTryStatement(hir));
294297
}
295298

@@ -317,7 +320,11 @@ function runWithEnvironment(
317320
value: hir,
318321
});
319322

320-
if (env.enableValidations && env.config.validateStaticComponents) {
323+
if (
324+
env.enableValidations &&
325+
env.config.validateStaticComponents &&
326+
env.outputMode === 'lint'
327+
) {
321328
env.logErrors(validateStaticComponents(hir));
322329
}
323330

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-conditionally-in-effect.expect.md

Lines changed: 15 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
## Input
33

44
```javascript
5-
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
5+
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly @outputMode:"lint"
66
import {useEffect, useState} from 'react';
77

88
function Component({value, enabled}) {
@@ -29,43 +29,21 @@ export const FIXTURE_ENTRYPOINT = {
2929
## Code
3030

3131
```javascript
32-
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
32+
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly @outputMode:"lint"
3333
import { useEffect, useState } from "react";
3434

35-
function Component(t0) {
36-
const $ = _c(6);
37-
const { value, enabled } = t0;
35+
function Component({ value, enabled }) {
3836
const [localValue, setLocalValue] = useState("");
39-
let t1;
40-
let t2;
41-
if ($[0] !== enabled || $[1] !== value) {
42-
t1 = () => {
43-
if (enabled) {
44-
setLocalValue(value);
45-
} else {
46-
setLocalValue("disabled");
47-
}
48-
};
49-
50-
t2 = [value, enabled];
51-
$[0] = enabled;
52-
$[1] = value;
53-
$[2] = t1;
54-
$[3] = t2;
55-
} else {
56-
t1 = $[2];
57-
t2 = $[3];
58-
}
59-
useEffect(t1, t2);
60-
let t3;
61-
if ($[4] !== localValue) {
62-
t3 = <div>{localValue}</div>;
63-
$[4] = localValue;
64-
$[5] = t3;
65-
} else {
66-
t3 = $[5];
67-
}
68-
return t3;
37+
38+
useEffect(() => {
39+
if (enabled) {
40+
setLocalValue(value);
41+
} else {
42+
setLocalValue("disabled");
43+
}
44+
}, [value, enabled]);
45+
46+
return <div>{localValue}</div>;
6947
}
7048

7149
export const FIXTURE_ENTRYPOINT = {
@@ -78,8 +56,8 @@ export const FIXTURE_ENTRYPOINT = {
7856
## Logs
7957

8058
```
81-
{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nProps: [value]\n\nData Flow Tree:\n└── value (Prop)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":9,"column":6,"index":244},"end":{"line":9,"column":19,"index":257},"filename":"derived-state-conditionally-in-effect.ts","identifierName":"setLocalValue"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null}
82-
{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":107},"end":{"line":16,"column":1,"index":378},"filename":"derived-state-conditionally-in-effect.ts"},"fnName":"Component","memoSlots":6,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0}
59+
{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nProps: [value]\n\nData Flow Tree:\n└── value (Prop)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":9,"column":6,"index":263},"end":{"line":9,"column":19,"index":276},"filename":"derived-state-conditionally-in-effect.ts","identifierName":"setLocalValue"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null}
60+
{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":126},"end":{"line":16,"column":1,"index":397},"filename":"derived-state-conditionally-in-effect.ts"},"fnName":"Component","memoSlots":6,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0}
8361
```
8462
8563
### Eval output

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-conditionally-in-effect.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
1+
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly @outputMode:"lint"
22
import {useEffect, useState} from 'react';
33

44
function Component({value, enabled}) {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-default-props.expect.md

Lines changed: 12 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
## Input
33

44
```javascript
5-
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
5+
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly @outputMode:"lint"
66
import {useEffect, useState} from 'react';
77

88
export default function Component({input = 'empty'}) {
@@ -26,38 +26,18 @@ export const FIXTURE_ENTRYPOINT = {
2626
## Code
2727

2828
```javascript
29-
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
29+
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly @outputMode:"lint"
3030
import { useEffect, useState } from "react";
3131

32-
export default function Component(t0) {
33-
const $ = _c(5);
34-
const { input: t1 } = t0;
35-
const input = t1 === undefined ? "empty" : t1;
32+
export default function Component({ input = "empty" }) {
3633
const [currInput, setCurrInput] = useState(input);
37-
let t2;
38-
let t3;
39-
if ($[0] !== input) {
40-
t2 = () => {
41-
setCurrInput(input + "local const");
42-
};
43-
t3 = [input, "local const"];
44-
$[0] = input;
45-
$[1] = t2;
46-
$[2] = t3;
47-
} else {
48-
t2 = $[1];
49-
t3 = $[2];
50-
}
51-
useEffect(t2, t3);
52-
let t4;
53-
if ($[3] !== currInput) {
54-
t4 = <div>{currInput}</div>;
55-
$[3] = currInput;
56-
$[4] = t4;
57-
} else {
58-
t4 = $[4];
59-
}
60-
return t4;
34+
const localConst = "local const";
35+
36+
useEffect(() => {
37+
setCurrInput(input + localConst);
38+
}, [input, localConst]);
39+
40+
return <div>{currInput}</div>;
6141
}
6242

6343
export const FIXTURE_ENTRYPOINT = {
@@ -70,8 +50,8 @@ export const FIXTURE_ENTRYPOINT = {
7050
## Logs
7151

7252
```
73-
{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nProps: [input]\n\nData Flow Tree:\n└── input (Prop)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":9,"column":4,"index":276},"end":{"line":9,"column":16,"index":288},"filename":"derived-state-from-default-props.ts","identifierName":"setCurrInput"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null}
74-
{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":15,"index":122},"end":{"line":13,"column":1,"index":372},"filename":"derived-state-from-default-props.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0}
53+
{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nProps: [input]\n\nData Flow Tree:\n└── input (Prop)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":9,"column":4,"index":295},"end":{"line":9,"column":16,"index":307},"filename":"derived-state-from-default-props.ts","identifierName":"setCurrInput"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null}
54+
{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":15,"index":141},"end":{"line":13,"column":1,"index":391},"filename":"derived-state-from-default-props.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0}
7555
```
7656
7757
### Eval output

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-default-props.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
1+
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly @outputMode:"lint"
22
import {useEffect, useState} from 'react';
33

44
export default function Component({input = 'empty'}) {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-local-state-in-effect.expect.md

Lines changed: 13 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
## Input
33

44
```javascript
5-
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
5+
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly @outputMode:"lint"
66

77
import {useEffect, useState} from 'react';
88

@@ -23,54 +23,29 @@ function Component({shouldChange}) {
2323
## Code
2424

2525
```javascript
26-
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
26+
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly @outputMode:"lint"
2727

2828
import { useEffect, useState } from "react";
2929

30-
function Component(t0) {
31-
const $ = _c(7);
32-
const { shouldChange } = t0;
30+
function Component({ shouldChange }) {
3331
const [count, setCount] = useState(0);
34-
let t1;
35-
if ($[0] !== count || $[1] !== shouldChange) {
36-
t1 = () => {
37-
if (shouldChange) {
38-
setCount(count + 1);
39-
}
40-
};
41-
$[0] = count;
42-
$[1] = shouldChange;
43-
$[2] = t1;
44-
} else {
45-
t1 = $[2];
46-
}
47-
let t2;
48-
if ($[3] !== count) {
49-
t2 = [count];
50-
$[3] = count;
51-
$[4] = t2;
52-
} else {
53-
t2 = $[4];
54-
}
55-
useEffect(t1, t2);
56-
let t3;
57-
if ($[5] !== count) {
58-
t3 = <div>{count}</div>;
59-
$[5] = count;
60-
$[6] = t3;
61-
} else {
62-
t3 = $[6];
63-
}
64-
return t3;
32+
33+
useEffect(() => {
34+
if (shouldChange) {
35+
setCount(count + 1);
36+
}
37+
}, [count]);
38+
39+
return <div>{count}</div>;
6540
}
6641

6742
```
6843

6944
## Logs
7045

7146
```
72-
{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nState: [count]\n\nData Flow Tree:\n└── count (State)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":10,"column":6,"index":237},"end":{"line":10,"column":14,"index":245},"filename":"derived-state-from-local-state-in-effect.ts","identifierName":"setCount"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null}
73-
{"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":108},"end":{"line":15,"column":1,"index":310},"filename":"derived-state-from-local-state-in-effect.ts"},"fnName":"Component","memoSlots":7,"memoBlocks":3,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0}
47+
{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nState: [count]\n\nData Flow Tree:\n└── count (State)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":10,"column":6,"index":256},"end":{"line":10,"column":14,"index":264},"filename":"derived-state-from-local-state-in-effect.ts","identifierName":"setCount"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null}
48+
{"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":127},"end":{"line":15,"column":1,"index":329},"filename":"derived-state-from-local-state-in-effect.ts"},"fnName":"Component","memoSlots":7,"memoBlocks":3,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0}
7449
```
7550
7651
### Eval output

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-local-state-in-effect.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
1+
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly @outputMode:"lint"
22

33
import {useEffect, useState} from 'react';
44

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-local-state-and-component-scope.expect.md

Lines changed: 18 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
## Input
33

44
```javascript
5-
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
5+
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly @outputMode:"lint"
66
import {useEffect, useState} from 'react';
77

88
function Component({firstName}) {
@@ -33,68 +33,25 @@ export const FIXTURE_ENTRYPOINT = {
3333
## Code
3434

3535
```javascript
36-
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
36+
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly @outputMode:"lint"
3737
import { useEffect, useState } from "react";
3838

39-
function Component(t0) {
40-
const $ = _c(12);
41-
const { firstName } = t0;
39+
function Component({ firstName }) {
4240
const [lastName, setLastName] = useState("Doe");
4341
const [fullName, setFullName] = useState("John");
44-
let t1;
45-
let t2;
46-
if ($[0] !== firstName || $[1] !== lastName) {
47-
t1 = () => {
48-
setFullName(firstName + " " + "D." + " " + lastName);
49-
};
50-
t2 = [firstName, "D.", lastName];
51-
$[0] = firstName;
52-
$[1] = lastName;
53-
$[2] = t1;
54-
$[3] = t2;
55-
} else {
56-
t1 = $[2];
57-
t2 = $[3];
58-
}
59-
useEffect(t1, t2);
60-
let t3;
61-
if ($[4] === Symbol.for("react.memo_cache_sentinel")) {
62-
t3 = (e) => setLastName(e.target.value);
63-
$[4] = t3;
64-
} else {
65-
t3 = $[4];
66-
}
67-
let t4;
68-
if ($[5] !== lastName) {
69-
t4 = <input value={lastName} onChange={t3} />;
70-
$[5] = lastName;
71-
$[6] = t4;
72-
} else {
73-
t4 = $[6];
74-
}
75-
let t5;
76-
if ($[7] !== fullName) {
77-
t5 = <div>{fullName}</div>;
78-
$[7] = fullName;
79-
$[8] = t5;
80-
} else {
81-
t5 = $[8];
82-
}
83-
let t6;
84-
if ($[9] !== t4 || $[10] !== t5) {
85-
t6 = (
86-
<div>
87-
{t4}
88-
{t5}
89-
</div>
90-
);
91-
$[9] = t4;
92-
$[10] = t5;
93-
$[11] = t6;
94-
} else {
95-
t6 = $[11];
96-
}
97-
return t6;
42+
43+
const middleName = "D.";
44+
45+
useEffect(() => {
46+
setFullName(firstName + " " + middleName + " " + lastName);
47+
}, [firstName, middleName, lastName]);
48+
49+
return (
50+
<div>
51+
<input value={lastName} onChange={(e) => setLastName(e.target.value)} />
52+
<div>{fullName}</div>
53+
</div>
54+
);
9855
}
9956

10057
export const FIXTURE_ENTRYPOINT = {
@@ -107,8 +64,8 @@ export const FIXTURE_ENTRYPOINT = {
10764
## Logs
10865

10966
```
110-
{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nProps: [firstName]\nState: [lastName]\n\nData Flow Tree:\n├── firstName (Prop)\n└── lastName (State)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":11,"column":4,"index":297},"end":{"line":11,"column":15,"index":308},"filename":"derived-state-from-prop-local-state-and-component-scope.ts","identifierName":"setFullName"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null}
111-
{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":107},"end":{"line":20,"column":1,"index":542},"filename":"derived-state-from-prop-local-state-and-component-scope.ts"},"fnName":"Component","memoSlots":12,"memoBlocks":5,"memoValues":6,"prunedMemoBlocks":0,"prunedMemoValues":0}
67+
{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nProps: [firstName]\nState: [lastName]\n\nData Flow Tree:\n├── firstName (Prop)\n└── lastName (State)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":11,"column":4,"index":316},"end":{"line":11,"column":15,"index":327},"filename":"derived-state-from-prop-local-state-and-component-scope.ts","identifierName":"setFullName"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null}
68+
{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":126},"end":{"line":20,"column":1,"index":561},"filename":"derived-state-from-prop-local-state-and-component-scope.ts"},"fnName":"Component","memoSlots":12,"memoBlocks":5,"memoValues":6,"prunedMemoBlocks":0,"prunedMemoValues":0}
11269
```
11370
11471
### Eval output

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-local-state-and-component-scope.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
1+
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly @outputMode:"lint"
22
import {useEffect, useState} from 'react';
33

44
function Component({firstName}) {

0 commit comments

Comments
 (0)