Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
7ab939bc55 |
@@ -511,6 +511,13 @@ type TreeNode = {
|
|||||||
children: Array<TreeNode>;
|
children: Array<TreeNode>;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
type DataFlowInfo = {
|
||||||
|
rootSources: string;
|
||||||
|
trees: Array<string>;
|
||||||
|
propsArr: Array<string>;
|
||||||
|
stateArr: Array<string>;
|
||||||
|
};
|
||||||
|
|
||||||
function buildTreeNode(
|
function buildTreeNode(
|
||||||
sourceId: IdentifierId,
|
sourceId: IdentifierId,
|
||||||
context: ValidationContext,
|
context: ValidationContext,
|
||||||
@@ -628,6 +635,51 @@ function renderTree(
|
|||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Builds the data flow information including trees and source categorization
|
||||||
|
*/
|
||||||
|
function buildDataFlowInfo(
|
||||||
|
sourceIds: Set<IdentifierId>,
|
||||||
|
context: ValidationContext,
|
||||||
|
): DataFlowInfo {
|
||||||
|
const propsSet = new Set<string>();
|
||||||
|
const stateSet = new Set<string>();
|
||||||
|
|
||||||
|
const rootNodesMap = new Map<string, TreeNode>();
|
||||||
|
for (const id of sourceIds) {
|
||||||
|
const nodes = buildTreeNode(id, context);
|
||||||
|
for (const node of nodes) {
|
||||||
|
if (!rootNodesMap.has(node.name)) {
|
||||||
|
rootNodesMap.set(node.name, node);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
const rootNodes = Array.from(rootNodesMap.values());
|
||||||
|
|
||||||
|
const trees = rootNodes.map((node, index) =>
|
||||||
|
renderTree(node, '', index === rootNodes.length - 1, propsSet, stateSet),
|
||||||
|
);
|
||||||
|
|
||||||
|
const propsArr = Array.from(propsSet);
|
||||||
|
const stateArr = Array.from(stateSet);
|
||||||
|
|
||||||
|
let rootSources = '';
|
||||||
|
if (propsArr.length > 0) {
|
||||||
|
rootSources += `Props: [${propsArr.join(', ')}]`;
|
||||||
|
}
|
||||||
|
if (stateArr.length > 0) {
|
||||||
|
if (rootSources) rootSources += '\n';
|
||||||
|
rootSources += `State: [${stateArr.join(', ')}]`;
|
||||||
|
}
|
||||||
|
|
||||||
|
return {
|
||||||
|
rootSources,
|
||||||
|
trees,
|
||||||
|
propsArr,
|
||||||
|
stateArr,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
function getFnLocalDeps(
|
function getFnLocalDeps(
|
||||||
fn: FunctionExpression | undefined,
|
fn: FunctionExpression | undefined,
|
||||||
): Set<IdentifierId> | undefined {
|
): Set<IdentifierId> | undefined {
|
||||||
@@ -792,47 +844,16 @@ function validateEffect(
|
|||||||
effectSetStateUsages.get(rootSetStateCall)!.size ===
|
effectSetStateUsages.get(rootSetStateCall)!.size ===
|
||||||
context.setStateUsages.get(rootSetStateCall)!.size - 1
|
context.setStateUsages.get(rootSetStateCall)!.size - 1
|
||||||
) {
|
) {
|
||||||
const propsSet = new Set<string>();
|
|
||||||
const stateSet = new Set<string>();
|
|
||||||
|
|
||||||
const rootNodesMap = new Map<string, TreeNode>();
|
|
||||||
for (const id of derivedSetStateCall.sourceIds) {
|
|
||||||
const nodes = buildTreeNode(id, context);
|
|
||||||
for (const node of nodes) {
|
|
||||||
if (!rootNodesMap.has(node.name)) {
|
|
||||||
rootNodesMap.set(node.name, node);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
const rootNodes = Array.from(rootNodesMap.values());
|
|
||||||
|
|
||||||
const trees = rootNodes.map((node, index) =>
|
|
||||||
renderTree(
|
|
||||||
node,
|
|
||||||
'',
|
|
||||||
index === rootNodes.length - 1,
|
|
||||||
propsSet,
|
|
||||||
stateSet,
|
|
||||||
),
|
|
||||||
);
|
|
||||||
|
|
||||||
for (const dep of derivedSetStateCall.sourceIds) {
|
for (const dep of derivedSetStateCall.sourceIds) {
|
||||||
if (cleanUpFunctionDeps !== undefined && cleanUpFunctionDeps.has(dep)) {
|
if (cleanUpFunctionDeps !== undefined && cleanUpFunctionDeps.has(dep)) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const propsArr = Array.from(propsSet);
|
const {rootSources, trees} = buildDataFlowInfo(
|
||||||
const stateArr = Array.from(stateSet);
|
derivedSetStateCall.sourceIds,
|
||||||
|
context,
|
||||||
let rootSources = '';
|
);
|
||||||
if (propsArr.length > 0) {
|
|
||||||
rootSources += `Props: [${propsArr.join(', ')}]`;
|
|
||||||
}
|
|
||||||
if (stateArr.length > 0) {
|
|
||||||
if (rootSources) rootSources += '\n';
|
|
||||||
rootSources += `State: [${stateArr.join(', ')}]`;
|
|
||||||
}
|
|
||||||
|
|
||||||
const description = `Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user
|
const description = `Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user
|
||||||
|
|
||||||
@@ -857,6 +878,80 @@ See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-o
|
|||||||
message: 'This should be computed during render, not in an effect',
|
message: 'This should be computed during render, not in an effect',
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
|
} else if (
|
||||||
|
rootSetStateCall !== null &&
|
||||||
|
effectSetStateUsages.has(rootSetStateCall) &&
|
||||||
|
context.setStateUsages.has(rootSetStateCall) &&
|
||||||
|
effectSetStateUsages.get(rootSetStateCall)!.size <
|
||||||
|
context.setStateUsages.get(rootSetStateCall)!.size
|
||||||
|
) {
|
||||||
|
for (const dep of derivedSetStateCall.sourceIds) {
|
||||||
|
if (cleanUpFunctionDeps !== undefined && cleanUpFunctionDeps.has(dep)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const {rootSources, trees} = buildDataFlowInfo(
|
||||||
|
derivedSetStateCall.sourceIds,
|
||||||
|
context,
|
||||||
|
);
|
||||||
|
|
||||||
|
// Find setState calls outside the effect
|
||||||
|
const allSetStateUsages = context.setStateUsages.get(rootSetStateCall)!;
|
||||||
|
const effectUsages = effectSetStateUsages.get(rootSetStateCall)!;
|
||||||
|
const outsideEffectUsages: Array<SourceLocation> = [];
|
||||||
|
|
||||||
|
for (const usage of allSetStateUsages) {
|
||||||
|
if (!effectUsages.has(usage)) {
|
||||||
|
outsideEffectUsages.push(usage);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const description = `Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user
|
||||||
|
|
||||||
|
This setState call is setting a derived value that depends on the following reactive sources:
|
||||||
|
|
||||||
|
${rootSources}
|
||||||
|
|
||||||
|
Data Flow Tree:
|
||||||
|
${trees.join('\n')}
|
||||||
|
|
||||||
|
This state is also being set outside of the effect. Consider hoisting the state up to a parent component and making this a controlled component.
|
||||||
|
|
||||||
|
See: https://react.dev/learn/sharing-state-between-components`;
|
||||||
|
|
||||||
|
const diagnosticDetails: Array<{
|
||||||
|
kind: 'error';
|
||||||
|
loc: SourceLocation;
|
||||||
|
message: string;
|
||||||
|
}> = [
|
||||||
|
{
|
||||||
|
kind: 'error',
|
||||||
|
loc: derivedSetStateCall.value.callee.loc,
|
||||||
|
message: 'setState in effect',
|
||||||
|
},
|
||||||
|
];
|
||||||
|
|
||||||
|
for (const usage of outsideEffectUsages) {
|
||||||
|
diagnosticDetails.push({
|
||||||
|
kind: 'error',
|
||||||
|
loc: usage,
|
||||||
|
message: 'setState outside effect',
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
let diagnostic = CompilerDiagnostic.create({
|
||||||
|
description: description,
|
||||||
|
category: ErrorCategory.EffectDerivationsOfState,
|
||||||
|
reason:
|
||||||
|
'Consider hoisting state to parent and making this a controlled component',
|
||||||
|
});
|
||||||
|
|
||||||
|
for (const detail of diagnosticDetails) {
|
||||||
|
diagnostic = diagnostic.withDetails(detail);
|
||||||
|
}
|
||||||
|
|
||||||
|
context.errors.pushDiagnostic(diagnostic);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user