Skip to content

Commit b229e26

Browse files
committed
[compiler] Improve setState-in-effects rule to account for ref-gated conditionals (#35147)
Conditionally calling setState in an effect is sometimes necessary, but should generally follow the pattern of using a "previous vaue" ref to manually compare and ensure that the setState is idempotent. See fixture for an example. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35147). * #35148 * __->__ #35147 DiffTrain build for [b946a24](b946a24)
1 parent 6e27e85 commit b229e26

35 files changed

+155
-88
lines changed

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

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51116,20 +51116,84 @@ function validateNoSetStateInEffects(fn, env) {
5111651116
return errors.asResult();
5111751117
}
5111851118
function getSetStateCall(fn, setStateFunctions, env) {
51119+
const enableAllowSetStateFromRefsInEffects = env.config.enableAllowSetStateFromRefsInEffects;
5111951120
const refDerivedValues = new Set();
5112051121
const isDerivedFromRef = (place) => {
5112151122
return (refDerivedValues.has(place.identifier.id) ||
5112251123
isUseRefType(place.identifier) ||
5112351124
isRefValueType(place.identifier));
5112451125
};
51126+
const isRefControlledBlock = enableAllowSetStateFromRefsInEffects
51127+
? createControlDominators(fn, place => isDerivedFromRef(place))
51128+
: () => false;
5112551129
for (const [, block] of fn.body.blocks) {
51130+
if (enableAllowSetStateFromRefsInEffects) {
51131+
for (const phi of block.phis) {
51132+
if (isDerivedFromRef(phi.place)) {
51133+
continue;
51134+
}
51135+
let isPhiDerivedFromRef = false;
51136+
for (const [, operand] of phi.operands) {
51137+
if (isDerivedFromRef(operand)) {
51138+
isPhiDerivedFromRef = true;
51139+
break;
51140+
}
51141+
}
51142+
if (isPhiDerivedFromRef) {
51143+
refDerivedValues.add(phi.place.identifier.id);
51144+
}
51145+
else {
51146+
for (const [pred] of phi.operands) {
51147+
if (isRefControlledBlock(pred)) {
51148+
refDerivedValues.add(phi.place.identifier.id);
51149+
break;
51150+
}
51151+
}
51152+
}
51153+
}
51154+
}
5112651155
for (const instr of block.instructions) {
51127-
if (env.config.enableAllowSetStateFromRefsInEffects) {
51156+
if (enableAllowSetStateFromRefsInEffects) {
5112851157
const hasRefOperand = Iterable_some(eachInstructionValueOperand(instr.value), isDerivedFromRef);
5112951158
if (hasRefOperand) {
5113051159
for (const lvalue of eachInstructionLValue(instr)) {
5113151160
refDerivedValues.add(lvalue.identifier.id);
5113251161
}
51162+
for (const operand of eachInstructionValueOperand(instr.value)) {
51163+
switch (operand.effect) {
51164+
case Effect.Capture:
51165+
case Effect.Store:
51166+
case Effect.ConditionallyMutate:
51167+
case Effect.ConditionallyMutateIterator:
51168+
case Effect.Mutate: {
51169+
if (isMutable(instr, operand)) {
51170+
refDerivedValues.add(operand.identifier.id);
51171+
}
51172+
break;
51173+
}
51174+
case Effect.Freeze:
51175+
case Effect.Read: {
51176+
break;
51177+
}
51178+
case Effect.Unknown: {
51179+
CompilerError.invariant(false, {
51180+
reason: 'Unexpected unknown effect',
51181+
description: null,
51182+
details: [
51183+
{
51184+
kind: 'error',
51185+
loc: operand.loc,
51186+
message: null,
51187+
},
51188+
],
51189+
suggestions: null,
51190+
});
51191+
}
51192+
default: {
51193+
assertExhaustive$1(operand.effect, `Unexpected effect kind \`${operand.effect}\``);
51194+
}
51195+
}
51196+
}
5113351197
}
5113451198
if (instr.value.kind === 'PropertyLoad' &&
5113551199
instr.value.property === 'current' &&
@@ -51156,13 +51220,16 @@ function getSetStateCall(fn, setStateFunctions, env) {
5115651220
const callee = instr.value.callee;
5115751221
if (isSetStateType(callee.identifier) ||
5115851222
setStateFunctions.has(callee.identifier.id)) {
51159-
if (env.config.enableAllowSetStateFromRefsInEffects) {
51223+
if (enableAllowSetStateFromRefsInEffects) {
5116051224
const arg = instr.value.args.at(0);
5116151225
if (arg !== undefined &&
5116251226
arg.kind === 'Identifier' &&
5116351227
refDerivedValues.has(arg.identifier.id)) {
5116451228
return null;
5116551229
}
51230+
else if (isRefControlledBlock(block.id)) {
51231+
continue;
51232+
}
5116651233
}
5116751234
return callee;
5116851235
}

compiled/facebook-www/REVISION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
d6b1a0573b4c43e5222aee1de5b11a8e4b575c8e
1+
b946a249b560a2d3afe1a1c8553d5491b1767cb3
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
d6b1a0573b4c43e5222aee1de5b11a8e4b575c8e
1+
b946a249b560a2d3afe1a1c8553d5491b1767cb3

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-d6b1a057-20251117";
1502+
exports.version = "19.3.0-www-classic-b946a249-20251117";
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-d6b1a057-20251117";
1502+
exports.version = "19.3.0-www-modern-b946a249-20251117";
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-d6b1a057-20251117";
609+
exports.version = "19.3.0-www-classic-b946a249-20251117";

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-d6b1a057-20251117";
609+
exports.version = "19.3.0-www-modern-b946a249-20251117";

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-d6b1a057-20251117";
613+
exports.version = "19.3.0-www-classic-b946a249-20251117";
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-d6b1a057-20251117";
613+
exports.version = "19.3.0-www-modern-b946a249-20251117";
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-d6b1a057-20251117",
20469+
version: "19.3.0-www-classic-b946a249-20251117",
2047020470
rendererPackageName: "react-art",
2047120471
currentDispatcherRef: ReactSharedInternals,
20472-
reconcilerVersion: "19.3.0-www-classic-d6b1a057-20251117"
20472+
reconcilerVersion: "19.3.0-www-classic-b946a249-20251117"
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-d6b1a057-20251117";
20507+
exports.version = "19.3.0-www-classic-b946a249-20251117";
2050820508
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
2050920509
"function" ===
2051020510
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

0 commit comments

Comments
 (0)