Skip to content

Commit 1434556

Browse files
committed
[Compiler] Avoid capturing global setStates for no-derived-computations lint (facebook#35135)
Summary: This only matters when enableTreatSetIdentifiersAsStateSetters=true This pattern is still bad. But Right now the validation can only recommend to move stuff to "calculate in render" A global setState should not be moved to render, not even conditionally and you can't remove state without crossing Component boundaries, which makes this a different kind of fix. So while we are only suggesting "calculate in render" as a fix we should disallow the lint from throwing in this case IMO Test Plan: Added a fixture --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35135). * __->__ facebook#35135 * facebook#35134 DiffTrain build for [257b033](facebook@257b033)
1 parent 23a8c95 commit 1434556

35 files changed

+114
-93
lines changed

compiled/eslint-plugin-react-hooks/index.js

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52418,16 +52418,18 @@ function isNamedIdentifier(place) {
5241852418
}
5241952419
function validateNoDerivedComputationsInEffects_exp(fn) {
5242052420
const functions = new Map();
52421+
const candidateDependencies = new Map();
5242152422
const derivationCache = new DerivationCache();
5242252423
const errors = new CompilerError();
52423-
const effects = new Set();
52424+
const effectsCache = new Map();
5242452425
const setStateLoads = new Map();
5242552426
const setStateUsages = new Map();
5242652427
const context = {
5242752428
functions,
52429+
candidateDependencies,
5242852430
errors,
5242952431
derivationCache,
52430-
effects,
52432+
effectsCache,
5243152433
setStateLoads,
5243252434
setStateUsages,
5243352435
};
@@ -52464,8 +52466,8 @@ function validateNoDerivedComputationsInEffects_exp(fn) {
5246452466
}
5246552467
context.derivationCache.checkForChanges();
5246652468
} while (context.derivationCache.snapshot());
52467-
for (const effect of effects) {
52468-
validateEffect(effect, context);
52469+
for (const [, effect] of effectsCache) {
52470+
validateEffect(effect.effect, effect.dependencies, context);
5246952471
}
5247052472
return errors.asResult();
5247152473
}
@@ -52549,8 +52551,12 @@ function recordInstructionDerivations(instr, context, isFirstPass) {
5254952551
value.args[0].kind === 'Identifier' &&
5255052552
value.args[1].kind === 'Identifier') {
5255152553
const effectFunction = context.functions.get(value.args[0].identifier.id);
52552-
if (effectFunction != null) {
52553-
context.effects.add(effectFunction.loweredFunc.func);
52554+
const deps = context.candidateDependencies.get(value.args[1].identifier.id);
52555+
if (effectFunction != null && deps != null) {
52556+
context.effectsCache.set(value.args[0].identifier.id, {
52557+
effect: effectFunction.loweredFunc.func,
52558+
dependencies: deps,
52559+
});
5255452560
}
5255552561
}
5255652562
else if (isUseStateType(lvalue.identifier) && value.args.length > 0) {
@@ -52559,6 +52565,9 @@ function recordInstructionDerivations(instr, context, isFirstPass) {
5255952565
return;
5256052566
}
5256152567
}
52568+
else if (value.kind === 'ArrayExpression') {
52569+
context.candidateDependencies.set(lvalue.identifier.id, value);
52570+
}
5256252571
for (const operand of eachInstructionOperand(instr)) {
5256352572
if (context.setStateLoads.has(operand.identifier.id)) {
5256452573
const rootSetStateId = getRootSetState(operand.identifier.id, context.setStateLoads);
@@ -52715,11 +52724,19 @@ function getFnLocalDeps(fn) {
5271552724
}
5271652725
return deps;
5271752726
}
52718-
function validateEffect(effectFunction, context) {
52727+
function validateEffect(effectFunction, dependencies, context) {
5271952728
var _a;
5272052729
const seenBlocks = new Set();
5272152730
const effectDerivedSetStateCalls = [];
5272252731
const effectSetStateUsages = new Map();
52732+
for (const dep of dependencies.elements) {
52733+
if (dep.kind === 'Identifier') {
52734+
const root = getRootSetState(dep.identifier.id, context.setStateLoads);
52735+
if (root !== null) {
52736+
effectSetStateUsages.set(root, new Set([dep.loc]));
52737+
}
52738+
}
52739+
}
5272352740
let cleanUpFunctionDeps;
5272452741
const globals = new Set();
5272552742
for (const block of effectFunction.body.blocks.values()) {
@@ -52749,6 +52766,10 @@ function validateEffect(effectFunction, context) {
5274952766
isSetStateType(instr.value.callee.identifier) &&
5275052767
instr.value.args.length === 1 &&
5275152768
instr.value.args[0].kind === 'Identifier') {
52769+
const calleeMetadata = context.derivationCache.cache.get(instr.value.callee.identifier.id);
52770+
if ((calleeMetadata === null || calleeMetadata === void 0 ? void 0 : calleeMetadata.typeOfValue) != 'fromState') {
52771+
continue;
52772+
}
5275252773
const argMetadata = context.derivationCache.cache.get(instr.value.args[0].identifier.id);
5275352774
if (argMetadata !== undefined) {
5275452775
effectDerivedSetStateCalls.push({

compiled/facebook-www/REVISION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
093b3246e162c2a6fd1a376506ed1144ed140226
1+
257b033fc7b0518e7b1db32ca24e2354933b9d0e
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
093b3246e162c2a6fd1a376506ed1144ed140226
1+
257b033fc7b0518e7b1db32ca24e2354933b9d0e

compiled/facebook-www/React-dev.classic.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1499,7 +1499,7 @@ __DEV__ &&
14991499
exports.useTransition = function () {
15001500
return resolveDispatcher().useTransition();
15011501
};
1502-
exports.version = "19.3.0-www-classic-093b3246-20251113";
1502+
exports.version = "19.3.0-www-classic-257b033f-20251113";
15031503
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
15041504
"function" ===
15051505
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

compiled/facebook-www/React-dev.modern.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1499,7 +1499,7 @@ __DEV__ &&
14991499
exports.useTransition = function () {
15001500
return resolveDispatcher().useTransition();
15011501
};
1502-
exports.version = "19.3.0-www-modern-093b3246-20251113";
1502+
exports.version = "19.3.0-www-modern-257b033f-20251113";
15031503
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
15041504
"function" ===
15051505
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

compiled/facebook-www/React-prod.classic.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -606,4 +606,4 @@ exports.useSyncExternalStore = function (
606606
exports.useTransition = function () {
607607
return ReactSharedInternals.H.useTransition();
608608
};
609-
exports.version = "19.3.0-www-classic-093b3246-20251113";
609+
exports.version = "19.3.0-www-classic-257b033f-20251113";

compiled/facebook-www/React-prod.modern.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -606,4 +606,4 @@ exports.useSyncExternalStore = function (
606606
exports.useTransition = function () {
607607
return ReactSharedInternals.H.useTransition();
608608
};
609-
exports.version = "19.3.0-www-modern-093b3246-20251113";
609+
exports.version = "19.3.0-www-modern-257b033f-20251113";

compiled/facebook-www/React-profiling.classic.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,7 @@ exports.useSyncExternalStore = function (
610610
exports.useTransition = function () {
611611
return ReactSharedInternals.H.useTransition();
612612
};
613-
exports.version = "19.3.0-www-classic-093b3246-20251113";
613+
exports.version = "19.3.0-www-classic-257b033f-20251113";
614614
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
615615
"function" ===
616616
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

compiled/facebook-www/React-profiling.modern.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,7 @@ exports.useSyncExternalStore = function (
610610
exports.useTransition = function () {
611611
return ReactSharedInternals.H.useTransition();
612612
};
613-
exports.version = "19.3.0-www-modern-093b3246-20251113";
613+
exports.version = "19.3.0-www-modern-257b033f-20251113";
614614
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
615615
"function" ===
616616
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

compiled/facebook-www/ReactART-dev.classic.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20466,10 +20466,10 @@ __DEV__ &&
2046620466
(function () {
2046720467
var internals = {
2046820468
bundleType: 1,
20469-
version: "19.3.0-www-classic-093b3246-20251113",
20469+
version: "19.3.0-www-classic-257b033f-20251113",
2047020470
rendererPackageName: "react-art",
2047120471
currentDispatcherRef: ReactSharedInternals,
20472-
reconcilerVersion: "19.3.0-www-classic-093b3246-20251113"
20472+
reconcilerVersion: "19.3.0-www-classic-257b033f-20251113"
2047320473
};
2047420474
internals.overrideHookState = overrideHookState;
2047520475
internals.overrideHookStateDeletePath = overrideHookStateDeletePath;
@@ -20504,7 +20504,7 @@ __DEV__ &&
2050420504
exports.Shape = Shape;
2050520505
exports.Surface = Surface;
2050620506
exports.Text = Text;
20507-
exports.version = "19.3.0-www-classic-093b3246-20251113";
20507+
exports.version = "19.3.0-www-classic-257b033f-20251113";
2050820508
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
2050920509
"function" ===
2051020510
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

0 commit comments

Comments
 (0)