Skip to content

Commit 889e084

Browse files
committed
[compiler] Improve merging of scopes that invalidate together (facebook#34049)
We try to merge consecutive reactive scopes that will always invalidate together, but there's one common case that isn't handled. ```js const y = [[x]]; ``` Here we'll create two consecutive scopes for the inner and outer array expressions. Because the input to the second scope is a temporary, they'll merge into one scope. But if we name the inner array, the merging stops: ```js const array = [x]; const y = [array]; ``` This is because the merging logic checks if all the dependencies of the second scope are outputs of the first scope, but doesn't account for renaming due to LoadLocal/StoreLocal. The fix is to track these temporaries. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34049). * __->__ facebook#34049 * facebook#34047 * facebook#34044 DiffTrain build for [ddf8bc3](facebook@ddf8bc3)
1 parent cfdfe3c commit 889e084

35 files changed

+270
-98
lines changed

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

Lines changed: 184 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18099,6 +18099,61 @@ function phiTypeEquals(tA, tB) {
1809918099
return false;
1810018100
}
1810118101

18102+
const RESERVED_WORDS = new Set([
18103+
'break',
18104+
'case',
18105+
'catch',
18106+
'class',
18107+
'const',
18108+
'continue',
18109+
'debugger',
18110+
'default',
18111+
'delete',
18112+
'do',
18113+
'else',
18114+
'enum',
18115+
'export',
18116+
'extends',
18117+
'false',
18118+
'finally',
18119+
'for',
18120+
'function',
18121+
'if',
18122+
'import',
18123+
'in',
18124+
'instanceof',
18125+
'new',
18126+
'null',
18127+
'return',
18128+
'super',
18129+
'switch',
18130+
'this',
18131+
'throw',
18132+
'true',
18133+
'try',
18134+
'typeof',
18135+
'var',
18136+
'void',
18137+
'while',
18138+
'with',
18139+
]);
18140+
const STRICT_MODE_RESERVED_WORDS = new Set([
18141+
'let',
18142+
'static',
18143+
'implements',
18144+
'interface',
18145+
'package',
18146+
'private',
18147+
'protected',
18148+
'public',
18149+
]);
18150+
const STRICT_MODE_RESTRICTED_WORDS = new Set(['eval', 'arguments']);
18151+
function isReservedWord(identifierName) {
18152+
return (RESERVED_WORDS.has(identifierName) ||
18153+
STRICT_MODE_RESERVED_WORDS.has(identifierName) ||
18154+
STRICT_MODE_RESTRICTED_WORDS.has(identifierName));
18155+
}
18156+
1810218157
const GeneratedSource = Symbol();
1810318158
function isStatementBlockKind(kind) {
1810418159
return kind === 'block' || kind === 'catch';
@@ -18156,12 +18211,22 @@ function forkTemporaryIdentifier(id, source) {
1815618211
return Object.assign(Object.assign({}, source), { mutableRange: { start: makeInstructionId(0), end: makeInstructionId(0) }, id });
1815718212
}
1815818213
function makeIdentifierName(name) {
18159-
CompilerError.invariant(libExports$1.isValidIdentifier(name), {
18160-
reason: `Expected a valid identifier name`,
18161-
loc: GeneratedSource,
18162-
description: `\`${name}\` is not a valid JavaScript identifier`,
18163-
suggestions: null,
18164-
});
18214+
if (isReservedWord(name)) {
18215+
CompilerError.throwInvalidJS({
18216+
reason: 'Expected a non-reserved identifier name',
18217+
loc: GeneratedSource,
18218+
description: `\`${name}\` is a reserved word in JavaScript and cannot be used as an identifier name`,
18219+
suggestions: null,
18220+
});
18221+
}
18222+
else {
18223+
CompilerError.invariant(libExports$1.isValidIdentifier(name), {
18224+
reason: `Expected a valid identifier name`,
18225+
loc: GeneratedSource,
18226+
description: `\`${name}\` is not a valid JavaScript identifier`,
18227+
suggestions: null,
18228+
});
18229+
}
1816518230
return {
1816618231
kind: 'named',
1816718232
value: name,
@@ -21297,6 +21362,7 @@ function parseAliasingSignatureConfig(typeConfig, moduleName, loc) {
2129721362
const temporaries = typeConfig.temporaries.map(define);
2129821363
const effects = typeConfig.effects.map((effect) => {
2129921364
switch (effect.kind) {
21365+
case 'ImmutableCapture':
2130021366
case 'CreateFrom':
2130121367
case 'Capture':
2130221368
case 'Alias':
@@ -29739,6 +29805,96 @@ const TYPED_GLOBALS = [
2973929805
returnValueKind: ValueKind.Mutable,
2974029806
}),
2974129807
],
29808+
[
29809+
'entries',
29810+
addFunction(DEFAULT_SHAPES, [], {
29811+
positionalParams: [Effect.Capture],
29812+
restParam: null,
29813+
returnType: { kind: 'Object', shapeId: BuiltInArrayId },
29814+
calleeEffect: Effect.Read,
29815+
returnValueKind: ValueKind.Mutable,
29816+
aliasing: {
29817+
receiver: '@receiver',
29818+
params: ['@object'],
29819+
rest: null,
29820+
returns: '@returns',
29821+
temporaries: [],
29822+
effects: [
29823+
{
29824+
kind: 'Create',
29825+
into: '@returns',
29826+
reason: ValueReason.KnownReturnSignature,
29827+
value: ValueKind.Mutable,
29828+
},
29829+
{
29830+
kind: 'Capture',
29831+
from: '@object',
29832+
into: '@returns',
29833+
},
29834+
],
29835+
},
29836+
}),
29837+
],
29838+
[
29839+
'keys',
29840+
addFunction(DEFAULT_SHAPES, [], {
29841+
positionalParams: [Effect.Read],
29842+
restParam: null,
29843+
returnType: { kind: 'Object', shapeId: BuiltInArrayId },
29844+
calleeEffect: Effect.Read,
29845+
returnValueKind: ValueKind.Mutable,
29846+
aliasing: {
29847+
receiver: '@receiver',
29848+
params: ['@object'],
29849+
rest: null,
29850+
returns: '@returns',
29851+
temporaries: [],
29852+
effects: [
29853+
{
29854+
kind: 'Create',
29855+
into: '@returns',
29856+
reason: ValueReason.KnownReturnSignature,
29857+
value: ValueKind.Mutable,
29858+
},
29859+
{
29860+
kind: 'ImmutableCapture',
29861+
from: '@object',
29862+
into: '@returns',
29863+
},
29864+
],
29865+
},
29866+
}),
29867+
],
29868+
[
29869+
'values',
29870+
addFunction(DEFAULT_SHAPES, [], {
29871+
positionalParams: [Effect.Capture],
29872+
restParam: null,
29873+
returnType: { kind: 'Object', shapeId: BuiltInArrayId },
29874+
calleeEffect: Effect.Read,
29875+
returnValueKind: ValueKind.Mutable,
29876+
aliasing: {
29877+
receiver: '@receiver',
29878+
params: ['@object'],
29879+
rest: null,
29880+
returns: '@returns',
29881+
temporaries: [],
29882+
effects: [
29883+
{
29884+
kind: 'Create',
29885+
into: '@returns',
29886+
reason: ValueReason.KnownReturnSignature,
29887+
value: ValueKind.Mutable,
29888+
},
29889+
{
29890+
kind: 'Capture',
29891+
from: '@object',
29892+
into: '@returns',
29893+
},
29894+
],
29895+
},
29896+
}),
29897+
],
2974229898
]),
2974329899
],
2974429900
[
@@ -30558,6 +30714,11 @@ const AliasEffectSchema = zod.z.object({
3055830714
from: LifetimeIdSchema,
3055930715
into: LifetimeIdSchema,
3056030716
});
30717+
const ImmutableCaptureEffectSchema = zod.z.object({
30718+
kind: zod.z.literal('ImmutableCapture'),
30719+
from: LifetimeIdSchema,
30720+
into: LifetimeIdSchema,
30721+
});
3056130722
const CaptureEffectSchema = zod.z.object({
3056230723
kind: zod.z.literal('Capture'),
3056330724
from: LifetimeIdSchema,
@@ -30597,6 +30758,7 @@ const AliasingEffectSchema = zod.z.union([
3059730758
AssignEffectSchema,
3059830759
AliasEffectSchema,
3059930760
CaptureEffectSchema,
30761+
ImmutableCaptureEffectSchema,
3060030762
ImpureEffectSchema,
3060130763
MutateEffectSchema,
3060230764
MutateTransitiveConditionallySchema,
@@ -37325,6 +37487,7 @@ class FindLastUsageVisitor extends ReactiveFunctionVisitor {
3732537487
let Transform$4 = class Transform extends ReactiveFunctionTransform {
3732637488
constructor(lastUsage) {
3732737489
super();
37490+
this.temporaries = new Map();
3732837491
this.lastUsage = lastUsage;
3732937492
}
3733037493
transformScope(scopeBlock, state) {
@@ -37338,6 +37501,7 @@ let Transform$4 = class Transform extends ReactiveFunctionTransform {
3733837501
}
3733937502
}
3734037503
visitBlock(block, state) {
37504+
var _a;
3734137505
this.traverseBlock(block, state);
3734237506
let current = null;
3734337507
const merged = [];
@@ -37381,6 +37545,9 @@ let Transform$4 = class Transform extends ReactiveFunctionTransform {
3738137545
case 'UnaryExpression': {
3738237546
if (current !== null && instr.instruction.lvalue !== null) {
3738337547
current.lvalues.add(instr.instruction.lvalue.identifier.declarationId);
37548+
if (instr.instruction.value.kind === 'LoadLocal') {
37549+
this.temporaries.set(instr.instruction.lvalue.identifier.declarationId, instr.instruction.value.place.identifier.declarationId);
37550+
}
3738437551
}
3738537552
break;
3738637553
}
@@ -37390,6 +37557,8 @@ let Transform$4 = class Transform extends ReactiveFunctionTransform {
3739037557
for (const lvalue of eachInstructionLValue(instr.instruction)) {
3739137558
current.lvalues.add(lvalue.identifier.declarationId);
3739237559
}
37560+
this.temporaries.set(instr.instruction.value.lvalue.place.identifier
37561+
.declarationId, (_a = this.temporaries.get(instr.instruction.value.value.identifier.declarationId)) !== null && _a !== void 0 ? _a : instr.instruction.value.value.identifier.declarationId);
3739337562
}
3739437563
else {
3739537564
reset();
@@ -37407,7 +37576,7 @@ let Transform$4 = class Transform extends ReactiveFunctionTransform {
3740737576
}
3740837577
case 'scope': {
3740937578
if (current !== null &&
37410-
canMergeScopes(current.block, instr) &&
37579+
canMergeScopes(current.block, instr, this.temporaries) &&
3741137580
areLValuesLastUsedByScope(instr.scope, current.lvalues, this.lastUsage)) {
3741237581
current.block.scope.range.end = makeInstructionId(Math.max(current.block.scope.range.end, instr.scope.range.end));
3741337582
for (const [key, value] of instr.scope.declarations) {
@@ -37503,7 +37672,7 @@ function areLValuesLastUsedByScope(scope, lvalues, lastUsage) {
3750337672
}
3750437673
return true;
3750537674
}
37506-
function canMergeScopes(current, next) {
37675+
function canMergeScopes(current, next, temporaries) {
3750737676
if (current.scope.reassignments.size !== 0 ||
3750837677
next.scope.reassignments.size !== 0) {
3750937678
return false;
@@ -37517,12 +37686,15 @@ function canMergeScopes(current, next) {
3751737686
path: [],
3751837687
}))), next.scope.dependencies) ||
3751937688
(next.scope.dependencies.size !== 0 &&
37520-
[...next.scope.dependencies].every(dep => isAlwaysInvalidatingType(dep.identifier.type) &&
37521-
Iterable_some(current.scope.declarations.values(), decl => decl.identifier.declarationId === dep.identifier.declarationId)))) {
37689+
[...next.scope.dependencies].every(dep => dep.path.length === 0 &&
37690+
isAlwaysInvalidatingType(dep.identifier.type) &&
37691+
Iterable_some(current.scope.declarations.values(), decl => decl.identifier.declarationId === dep.identifier.declarationId ||
37692+
decl.identifier.declarationId ===
37693+
temporaries.get(dep.identifier.declarationId))))) {
3752237694
return true;
3752337695
}
37524-
log(` ${printReactiveScopeSummary(current.scope)}`);
37525-
log(` ${printReactiveScopeSummary(next.scope)}`);
37696+
log(` ${printReactiveScopeSummary(current.scope)} ${[...current.scope.declarations.values()].map(decl => decl.identifier.declarationId)}`);
37697+
log(` ${printReactiveScopeSummary(next.scope)} ${[...next.scope.dependencies].map(dep => { var _a; return `${dep.identifier.declarationId} ${(_a = temporaries.get(dep.identifier.declarationId)) !== null && _a !== void 0 ? _a : dep.identifier.declarationId}`; })}`);
3752637698
return false;
3752737699
}
3752837700
function isAlwaysInvalidatingType(type) {

compiled/facebook-www/REVISION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
8de7aed8927d87a9e7e838f5d8ae28d5c30805d3
1+
ddf8bc3fbac7aefbf557e2e4a3e14d8de1b80872
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
8de7aed8927d87a9e7e838f5d8ae28d5c30805d3
1+
ddf8bc3fbac7aefbf557e2e4a3e14d8de1b80872

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-8de7aed8-20250730";
1437+
exports.version = "19.2.0-www-classic-ddf8bc3f-20250801";
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-8de7aed8-20250730";
1437+
exports.version = "19.2.0-www-modern-ddf8bc3f-20250801";
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-8de7aed8-20250730";
613+
exports.version = "19.2.0-www-classic-ddf8bc3f-20250801";

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-8de7aed8-20250730";
613+
exports.version = "19.2.0-www-modern-ddf8bc3f-20250801";

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-8de7aed8-20250730";
617+
exports.version = "19.2.0-www-classic-ddf8bc3f-20250801";
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-8de7aed8-20250730";
617+
exports.version = "19.2.0-www-modern-ddf8bc3f-20250801";
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-8de7aed8-20250730",
19321+
version: "19.2.0-www-classic-ddf8bc3f-20250801",
1932219322
rendererPackageName: "react-art",
1932319323
currentDispatcherRef: ReactSharedInternals,
19324-
reconcilerVersion: "19.2.0-www-classic-8de7aed8-20250730"
19324+
reconcilerVersion: "19.2.0-www-classic-ddf8bc3f-20250801"
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-8de7aed8-20250730";
19358+
exports.version = "19.2.0-www-classic-ddf8bc3f-20250801";
1935919359
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
1936019360
"function" ===
1936119361
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

0 commit comments

Comments
 (0)