mirror of
https://github.com/facebook/react.git
synced 2025-11-01 09:12:30 +00:00
[Compiler] Improve error for calculate in render useEffect validation
Summary: Change error and update snapshots The error now mentions what values are causing the issue which should provide better context on how to fix the issue
This commit is contained in:
+33
-9
@@ -5,7 +5,7 @@
|
||||
* LICENSE file in the root directory of this source tree.
|
||||
*/
|
||||
|
||||
import {CompilerError, Effect} from '..';
|
||||
import {CompilerDiagnostic, CompilerError, Effect} from '..';
|
||||
import {ErrorCategory} from '../CompilerError';
|
||||
import {
|
||||
BlockId,
|
||||
@@ -377,6 +377,7 @@ function validateEffect(
|
||||
value: CallExpression;
|
||||
loc: SourceLocation;
|
||||
sourceIds: Set<IdentifierId>;
|
||||
typeOfValue: TypeOfValue;
|
||||
}> = [];
|
||||
|
||||
const globals: Set<IdentifierId> = new Set();
|
||||
@@ -426,6 +427,7 @@ function validateEffect(
|
||||
value: instr.value,
|
||||
loc: instr.value.callee.loc,
|
||||
sourceIds: argMetadata.sourcesIds,
|
||||
typeOfValue: argMetadata.typeOfValue,
|
||||
});
|
||||
}
|
||||
} else if (instr.value.kind === 'CallExpression') {
|
||||
@@ -467,14 +469,36 @@ function validateEffect(
|
||||
.length -
|
||||
1
|
||||
) {
|
||||
context.errors.push({
|
||||
category: ErrorCategory.EffectDerivationsOfState,
|
||||
reason:
|
||||
'Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)',
|
||||
description: null,
|
||||
loc: derivedSetStateCall.value.callee.loc,
|
||||
suggestions: null,
|
||||
});
|
||||
const derivedDepsStr = Array.from(derivedSetStateCall.sourceIds)
|
||||
.map(sourceId => {
|
||||
const sourceMetadata = context.derivationCache.cache.get(sourceId);
|
||||
return sourceMetadata?.place.identifier.name?.value;
|
||||
})
|
||||
.filter(Boolean)
|
||||
.join(', ');
|
||||
|
||||
let description;
|
||||
|
||||
if (derivedSetStateCall.typeOfValue === 'fromProps') {
|
||||
description = `From props: [${derivedDepsStr}]`;
|
||||
} else if (derivedSetStateCall.typeOfValue === 'fromState') {
|
||||
description = `From local state: [${derivedDepsStr}]`;
|
||||
} else {
|
||||
description = `From props and local state: [${derivedDepsStr}]`;
|
||||
}
|
||||
|
||||
context.errors.pushDiagnostic(
|
||||
CompilerDiagnostic.create({
|
||||
description: `Derived values (${description}) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user`,
|
||||
category: ErrorCategory.EffectDerivationsOfState,
|
||||
reason:
|
||||
'You might not need an effect. Derive values in render, not effects.',
|
||||
}).withDetails({
|
||||
kind: 'error',
|
||||
loc: derivedSetStateCall.value.callee.loc,
|
||||
message: 'This should be computed during render, not in an effect',
|
||||
}),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+4
-2
@@ -32,13 +32,15 @@ export const FIXTURE_ENTRYPOINT = {
|
||||
```
|
||||
Found 1 error:
|
||||
|
||||
Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
Error: You might not need an effect. Derive values in render, not effects.
|
||||
|
||||
Derived values (From props: [value]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
|
||||
|
||||
error.derived-state-conditionally-in-effect.ts:9:6
|
||||
7 | useEffect(() => {
|
||||
8 | if (enabled) {
|
||||
> 9 | setLocalValue(value);
|
||||
| ^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
| ^^^^^^^^^^^^^ This should be computed during render, not in an effect
|
||||
10 | } else {
|
||||
11 | setLocalValue('disabled');
|
||||
12 | }
|
||||
|
||||
+4
-2
@@ -29,13 +29,15 @@ export const FIXTURE_ENTRYPOINT = {
|
||||
```
|
||||
Found 1 error:
|
||||
|
||||
Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
Error: You might not need an effect. Derive values in render, not effects.
|
||||
|
||||
Derived values (From props: [input]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
|
||||
|
||||
error.derived-state-from-default-props.ts:9:4
|
||||
7 |
|
||||
8 | useEffect(() => {
|
||||
> 9 | setCurrInput(input + localConst);
|
||||
| ^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
| ^^^^^^^^^^^^ This should be computed during render, not in an effect
|
||||
10 | }, [input, localConst]);
|
||||
11 |
|
||||
12 | return <div>{currInput}</div>;
|
||||
|
||||
+4
-2
@@ -26,13 +26,15 @@ function Component({shouldChange}) {
|
||||
```
|
||||
Found 1 error:
|
||||
|
||||
Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
Error: You might not need an effect. Derive values in render, not effects.
|
||||
|
||||
Derived values (From local state: [count]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
|
||||
|
||||
error.derived-state-from-local-state-in-effect.ts:10:6
|
||||
8 | useEffect(() => {
|
||||
9 | if (shouldChange) {
|
||||
> 10 | setCount(count + 1);
|
||||
| ^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
| ^^^^^^^^ This should be computed during render, not in an effect
|
||||
11 | }
|
||||
12 | }, [count]);
|
||||
13 |
|
||||
|
||||
+4
-2
@@ -36,13 +36,15 @@ export const FIXTURE_ENTRYPOINT = {
|
||||
```
|
||||
Found 1 error:
|
||||
|
||||
Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
Error: You might not need an effect. Derive values in render, not effects.
|
||||
|
||||
Derived values (From props and local state: [firstName, lastName]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
|
||||
|
||||
error.derived-state-from-prop-local-state-and-component-scope.ts:11:4
|
||||
9 |
|
||||
10 | useEffect(() => {
|
||||
> 11 | setFullName(firstName + ' ' + middleName + ' ' + lastName);
|
||||
| ^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
| ^^^^^^^^^^^ This should be computed during render, not in an effect
|
||||
12 | }, [firstName, middleName, lastName]);
|
||||
13 |
|
||||
14 | return (
|
||||
|
||||
+4
-2
@@ -29,13 +29,15 @@ export const FIXTURE_ENTRYPOINT = {
|
||||
```
|
||||
Found 1 error:
|
||||
|
||||
Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
Error: You might not need an effect. Derive values in render, not effects.
|
||||
|
||||
Derived values (From props: [value]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
|
||||
|
||||
error.derived-state-from-prop-with-side-effect.ts:8:4
|
||||
6 |
|
||||
7 | useEffect(() => {
|
||||
> 8 | setLocalValue(value);
|
||||
| ^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
| ^^^^^^^^^^^^^ This should be computed during render, not in an effect
|
||||
9 | document.title = `Value: ${value}`;
|
||||
10 | }, [value]);
|
||||
11 |
|
||||
|
||||
+4
-2
@@ -33,13 +33,15 @@ export const FIXTURE_ENTRYPOINT = {
|
||||
```
|
||||
Found 1 error:
|
||||
|
||||
Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
Error: You might not need an effect. Derive values in render, not effects.
|
||||
|
||||
Derived values (From props: [propValue]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
|
||||
|
||||
error.effect-contains-local-function-call.ts:12:4
|
||||
10 |
|
||||
11 | useEffect(() => {
|
||||
> 12 | setValue(propValue);
|
||||
| ^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
| ^^^^^^^^ This should be computed during render, not in an effect
|
||||
13 | localFunction();
|
||||
14 | }, [propValue]);
|
||||
15 |
|
||||
|
||||
+4
-2
@@ -31,13 +31,15 @@ export const FIXTURE_ENTRYPOINT = {
|
||||
```
|
||||
Found 1 error:
|
||||
|
||||
Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
Error: You might not need an effect. Derive values in render, not effects.
|
||||
|
||||
Derived values (From local state: [firstName]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
|
||||
|
||||
error.invalid-derived-computation-in-effect.ts:11:4
|
||||
9 | const [fullName, setFullName] = useState('');
|
||||
10 | useEffect(() => {
|
||||
> 11 | setFullName(firstName + ' ' + lastName);
|
||||
| ^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
| ^^^^^^^^^^^ This should be computed during render, not in an effect
|
||||
12 | }, [firstName, lastName]);
|
||||
13 |
|
||||
14 | return <div>{fullName}</div>;
|
||||
|
||||
+4
-2
@@ -29,13 +29,15 @@ export const FIXTURE_ENTRYPOINT = {
|
||||
```
|
||||
Found 1 error:
|
||||
|
||||
Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
Error: You might not need an effect. Derive values in render, not effects.
|
||||
|
||||
Derived values (From props: [props]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
|
||||
|
||||
error.invalid-derived-state-from-computed-props.ts:9:4
|
||||
7 | useEffect(() => {
|
||||
8 | const computed = props.prefix + props.value + props.suffix;
|
||||
> 9 | setDisplayValue(computed);
|
||||
| ^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
| ^^^^^^^^^^^^^^^ This should be computed during render, not in an effect
|
||||
10 | }, [props.prefix, props.value, props.suffix]);
|
||||
11 |
|
||||
12 | return <div>{displayValue}</div>;
|
||||
|
||||
+4
-2
@@ -30,13 +30,15 @@ export const FIXTURE_ENTRYPOINT = {
|
||||
```
|
||||
Found 1 error:
|
||||
|
||||
Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
Error: You might not need an effect. Derive values in render, not effects.
|
||||
|
||||
Derived values (From props: [props]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
|
||||
|
||||
error.invalid-derived-state-from-destructured-props.ts:10:4
|
||||
8 |
|
||||
9 | useEffect(() => {
|
||||
> 10 | setFullName(props.firstName + ' ' + props.lastName);
|
||||
| ^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
|
||||
| ^^^^^^^^^^^ This should be computed during render, not in an effect
|
||||
11 | }, [props.firstName, props.lastName]);
|
||||
12 |
|
||||
13 | return <div>{fullName}</div>;
|
||||
|
||||
Reference in New Issue
Block a user