Skip to content

Commit 880a03f

Browse files
committed
[compiler] Allow assigning ref-accessing functions to objects if not mutated (#34026)
Allows assigning a ref-accessing function to an object so long as that object is not subsequently transitively mutated. We should likely rewrite the ref validation to use the new mutation/aliasing effects, which would provide a more consistent behavior across instruction types and require fewer special cases like this. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34026). * #34027 * __->__ #34026 DiffTrain build for [04a7a61](04a7a61)
1 parent e0a3dc5 commit 880a03f

35 files changed

+162
-103
lines changed

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

Lines changed: 76 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -45289,7 +45289,7 @@ function makeOrMergeProperty(node, property, accessType) {
4528945289
var _DependencyCollectionContext_instances, _DependencyCollectionContext_declarations, _DependencyCollectionContext_reassignments, _DependencyCollectionContext_scopes, _DependencyCollectionContext_dependencies, _DependencyCollectionContext_temporaries, _DependencyCollectionContext_temporariesUsedOutsideScope, _DependencyCollectionContext_processedInstrsInOptional, _DependencyCollectionContext_innerFnContext, _DependencyCollectionContext_checkValidDependency, _DependencyCollectionContext_isScopeActive;
4529045290
function propagateScopeDependenciesHIR(fn) {
4529145291
const usedOutsideDeclaringScope = findTemporariesUsedOutsideDeclaringScope(fn);
45292-
const temporaries = collectTemporariesSidemap(fn, usedOutsideDeclaringScope);
45292+
const temporaries = collectTemporariesSidemap$1(fn, usedOutsideDeclaringScope);
4529345293
const { temporariesReadInOptional, processedInstrsInOptional, hoistableObjects, } = collectOptionalChainSidemap(fn);
4529445294
const hoistablePropertyLoads = keyByScopeId(fn, collectHoistablePropertyLoads(fn, temporaries, hoistableObjects));
4529545295
const scopeDeps = collectDependencies(fn, usedOutsideDeclaringScope, new Map([...temporaries, ...temporariesReadInOptional]), processedInstrsInOptional);
@@ -45360,7 +45360,7 @@ function findTemporariesUsedOutsideDeclaringScope(fn) {
4536045360
}
4536145361
return usedOutsideDeclaringScope;
4536245362
}
45363-
function collectTemporariesSidemap(fn, usedOutsideDeclaringScope) {
45363+
function collectTemporariesSidemap$1(fn, usedOutsideDeclaringScope) {
4536445364
const temporaries = new Map();
4536545365
collectTemporariesSidemapImpl(fn, usedOutsideDeclaringScope, temporaries, null);
4536645366
return temporaries;
@@ -46185,7 +46185,7 @@ function collectDepUsages(deps, fnExpr) {
4618546185
}
4618646186
function inferMinimalDependencies(fnInstr) {
4618746187
const fn = fnInstr.value.loweredFunc.func;
46188-
const temporaries = collectTemporariesSidemap(fn, new Set());
46188+
const temporaries = collectTemporariesSidemap$1(fn, new Set());
4618946189
const { hoistableObjects, processedInstrsInOptional, temporariesReadInOptional, } = collectOptionalChainSidemap(fn);
4619046190
const hoistablePropertyLoads = collectHoistablePropertyLoadsInInnerFn(fnInstr, temporaries, hoistableObjects);
4619146191
const hoistableToFnEntry = hoistablePropertyLoads.get(fn.body.entry);
@@ -48151,7 +48151,7 @@ function validateNoCapitalizedCalls(fn) {
4815148151
return errors.asResult();
4815248152
}
4815348153

48154-
var _Env_changed;
48154+
var _Env_changed, _Env_data, _Env_temporaries;
4815548155
function makeRefId(id) {
4815648156
CompilerError.invariant(id >= 0 && Number.isInteger(id), {
4815748157
reason: 'Expected identifier id to be a non-negative integer',
@@ -48165,32 +48165,84 @@ let _refId = 0;
4816548165
function nextRefId() {
4816648166
return makeRefId(_refId++);
4816748167
}
48168-
class Env extends Map {
48168+
class Env {
4816948169
constructor() {
48170-
super(...arguments);
4817148170
_Env_changed.set(this, false);
48171+
_Env_data.set(this, new Map());
48172+
_Env_temporaries.set(this, new Map());
48173+
}
48174+
lookup(place) {
48175+
var _a;
48176+
return (_a = __classPrivateFieldGet(this, _Env_temporaries, "f").get(place.identifier.id)) !== null && _a !== void 0 ? _a : place;
48177+
}
48178+
define(place, value) {
48179+
__classPrivateFieldGet(this, _Env_temporaries, "f").set(place.identifier.id, value);
4817248180
}
4817348181
resetChanged() {
4817448182
__classPrivateFieldSet(this, _Env_changed, false, "f");
4817548183
}
4817648184
hasChanged() {
4817748185
return __classPrivateFieldGet(this, _Env_changed, "f");
4817848186
}
48187+
get(key) {
48188+
var _a, _b;
48189+
const operandId = (_b = (_a = __classPrivateFieldGet(this, _Env_temporaries, "f").get(key)) === null || _a === void 0 ? void 0 : _a.identifier.id) !== null && _b !== void 0 ? _b : key;
48190+
return __classPrivateFieldGet(this, _Env_data, "f").get(operandId);
48191+
}
4817948192
set(key, value) {
48180-
const cur = this.get(key);
48193+
var _a, _b;
48194+
const operandId = (_b = (_a = __classPrivateFieldGet(this, _Env_temporaries, "f").get(key)) === null || _a === void 0 ? void 0 : _a.identifier.id) !== null && _b !== void 0 ? _b : key;
48195+
const cur = __classPrivateFieldGet(this, _Env_data, "f").get(operandId);
4818148196
const widenedValue = joinRefAccessTypes(value, cur !== null && cur !== void 0 ? cur : { kind: 'None' });
4818248197
if (!(cur == null && widenedValue.kind === 'None') &&
4818348198
(cur == null || !tyEqual(cur, widenedValue))) {
4818448199
__classPrivateFieldSet(this, _Env_changed, true, "f");
4818548200
}
48186-
return super.set(key, widenedValue);
48201+
__classPrivateFieldGet(this, _Env_data, "f").set(operandId, widenedValue);
48202+
return this;
4818748203
}
4818848204
}
48189-
_Env_changed = new WeakMap();
48205+
_Env_changed = new WeakMap(), _Env_data = new WeakMap(), _Env_temporaries = new WeakMap();
4819048206
function validateNoRefAccessInRender(fn) {
4819148207
const env = new Env();
48208+
collectTemporariesSidemap(fn, env);
4819248209
return validateNoRefAccessInRenderImpl(fn, env).map(_ => undefined);
4819348210
}
48211+
function collectTemporariesSidemap(fn, env) {
48212+
for (const block of fn.body.blocks.values()) {
48213+
for (const instr of block.instructions) {
48214+
const { lvalue, value } = instr;
48215+
switch (value.kind) {
48216+
case 'LoadLocal': {
48217+
const temp = env.lookup(value.place);
48218+
if (temp != null) {
48219+
env.define(lvalue, temp);
48220+
}
48221+
break;
48222+
}
48223+
case 'StoreLocal': {
48224+
const temp = env.lookup(value.value);
48225+
if (temp != null) {
48226+
env.define(lvalue, temp);
48227+
env.define(value.lvalue.place, temp);
48228+
}
48229+
break;
48230+
}
48231+
case 'PropertyLoad': {
48232+
if (isUseRefType(value.object.identifier) &&
48233+
value.property === 'current') {
48234+
continue;
48235+
}
48236+
const temp = env.lookup(value.object);
48237+
if (temp != null) {
48238+
env.define(lvalue, temp);
48239+
}
48240+
break;
48241+
}
48242+
}
48243+
}
48244+
}
48245+
}
4819448246
function refTypeOfType(place) {
4819548247
if (isRefValueType(place.identifier)) {
4819648248
return { kind: 'RefValue' };
@@ -48517,11 +48569,21 @@ function validateNoRefAccessInRenderImpl(fn, env) {
4851748569
else {
4851848570
validateNoRefUpdate(errors, env, instr.value.object, instr.loc);
4851948571
}
48520-
for (const operand of eachInstructionValueOperand(instr.value)) {
48521-
if (operand === instr.value.object) {
48522-
continue;
48572+
if (instr.value.kind === 'ComputedDelete' ||
48573+
instr.value.kind === 'ComputedStore') {
48574+
validateNoRefValueAccess(errors, env, instr.value.property);
48575+
}
48576+
if (instr.value.kind === 'ComputedStore' ||
48577+
instr.value.kind === 'PropertyStore') {
48578+
validateNoDirectRefValueAccess(errors, instr.value.value, env);
48579+
const type = env.get(instr.value.value.identifier.id);
48580+
if (type != null && type.kind === 'Structure') {
48581+
let objectType = type;
48582+
if (target != null) {
48583+
objectType = joinRefAccessTypes(objectType, target);
48584+
}
48585+
env.set(instr.value.object.identifier.id, objectType);
4852348586
}
48524-
validateNoRefValueAccess(errors, env, operand);
4852548587
}
4852648588
break;
4852748589
}
@@ -48665,11 +48727,8 @@ function validateNoRefPassedToFunction(errors, env, operand, loc) {
4866548727
}
4866648728
}
4866748729
function validateNoRefUpdate(errors, env, operand, loc) {
48668-
var _a;
4866948730
const type = destructure(env.get(operand.identifier.id));
48670-
if ((type === null || type === void 0 ? void 0 : type.kind) === 'Ref' ||
48671-
(type === null || type === void 0 ? void 0 : type.kind) === 'RefValue' ||
48672-
((type === null || type === void 0 ? void 0 : type.kind) === 'Structure' && ((_a = type.fn) === null || _a === void 0 ? void 0 : _a.readRefEffect))) {
48731+
if ((type === null || type === void 0 ? void 0 : type.kind) === 'Ref' || (type === null || type === void 0 ? void 0 : type.kind) === 'RefValue') {
4867348732
errors.pushDiagnostic(CompilerDiagnostic.create({
4867448733
severity: ErrorSeverity.InvalidReact,
4867548734
category: 'Cannot access refs during render',

compiled/facebook-www/REVISION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
c2326b1336e38a52899fad6c2ddbb71ea7ddd3ee
1+
04a7a61918522734a083bff37843865d7815d466
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
c2326b1336e38a52899fad6c2ddbb71ea7ddd3ee
1+
04a7a61918522734a083bff37843865d7815d466

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1434,7 +1434,7 @@ __DEV__ &&
14341434
exports.useTransition = function () {
14351435
return resolveDispatcher().useTransition();
14361436
};
1437-
exports.version = "19.2.0-www-classic-c2326b13-20250729";
1437+
exports.version = "19.2.0-www-classic-04a7a619-20250729";
14381438
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
14391439
"function" ===
14401440
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
@@ -1434,7 +1434,7 @@ __DEV__ &&
14341434
exports.useTransition = function () {
14351435
return resolveDispatcher().useTransition();
14361436
};
1437-
exports.version = "19.2.0-www-modern-c2326b13-20250729";
1437+
exports.version = "19.2.0-www-modern-04a7a619-20250729";
14381438
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
14391439
"function" ===
14401440
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
@@ -610,4 +610,4 @@ exports.useSyncExternalStore = function (
610610
exports.useTransition = function () {
611611
return ReactSharedInternals.H.useTransition();
612612
};
613-
exports.version = "19.2.0-www-classic-c2326b13-20250729";
613+
exports.version = "19.2.0-www-classic-04a7a619-20250729";

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -610,4 +610,4 @@ exports.useSyncExternalStore = function (
610610
exports.useTransition = function () {
611611
return ReactSharedInternals.H.useTransition();
612612
};
613-
exports.version = "19.2.0-www-modern-c2326b13-20250729";
613+
exports.version = "19.2.0-www-modern-04a7a619-20250729";

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ exports.useSyncExternalStore = function (
614614
exports.useTransition = function () {
615615
return ReactSharedInternals.H.useTransition();
616616
};
617-
exports.version = "19.2.0-www-classic-c2326b13-20250729";
617+
exports.version = "19.2.0-www-classic-04a7a619-20250729";
618618
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
619619
"function" ===
620620
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
@@ -614,7 +614,7 @@ exports.useSyncExternalStore = function (
614614
exports.useTransition = function () {
615615
return ReactSharedInternals.H.useTransition();
616616
};
617-
exports.version = "19.2.0-www-modern-c2326b13-20250729";
617+
exports.version = "19.2.0-www-modern-04a7a619-20250729";
618618
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
619619
"function" ===
620620
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
@@ -19318,10 +19318,10 @@ __DEV__ &&
1931819318
(function () {
1931919319
var internals = {
1932019320
bundleType: 1,
19321-
version: "19.2.0-www-classic-c2326b13-20250729",
19321+
version: "19.2.0-www-classic-04a7a619-20250729",
1932219322
rendererPackageName: "react-art",
1932319323
currentDispatcherRef: ReactSharedInternals,
19324-
reconcilerVersion: "19.2.0-www-classic-c2326b13-20250729"
19324+
reconcilerVersion: "19.2.0-www-classic-04a7a619-20250729"
1932519325
};
1932619326
internals.overrideHookState = overrideHookState;
1932719327
internals.overrideHookStateDeletePath = overrideHookStateDeletePath;
@@ -19355,7 +19355,7 @@ __DEV__ &&
1935519355
exports.Shape = Shape;
1935619356
exports.Surface = Surface;
1935719357
exports.Text = Text;
19358-
exports.version = "19.2.0-www-classic-c2326b13-20250729";
19358+
exports.version = "19.2.0-www-classic-04a7a619-20250729";
1935919359
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
1936019360
"function" ===
1936119361
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

0 commit comments

Comments
 (0)