Skip to content

Commit 1f49af9

Browse files
committed
Fix: Move destroy field to shared instance object (#26561)
This reverts commit d41e5c2. DiffTrain build for [1d3ffd9](1d3ffd9)
1 parent 38b20a3 commit 1f49af9

18 files changed

+684
-450
lines changed

compiled/facebook-www/REVISION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
2f18eb0ac63fdcaed3f1d4e3c375a559a552c09a
1+
1d3ffd9d9addb4aa0988b92ee6917c07c3c261ae

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if (
2727
}
2828
"use strict";
2929

30-
var ReactVersion = "18.3.0-www-modern-b6a03dc1";
30+
var ReactVersion = "18.3.0-www-modern-c1d269df";
3131

3232
// ATTENTION
3333
// When adding new symbols to this file,

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

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
6969
return self;
7070
}
7171

72-
var ReactVersion = "18.3.0-www-classic-ccedb28a";
72+
var ReactVersion = "18.3.0-www-classic-cd98b4c3";
7373

7474
var LegacyRoot = 0;
7575
var ConcurrentRoot = 1;
@@ -7343,7 +7343,21 @@ var didWarnAboutUseWrappedInTryCatch;
73437343
{
73447344
didWarnAboutMismatchedHooksForComponent = new Set();
73457345
didWarnAboutUseWrappedInTryCatch = new Set();
7346-
} // These are set right before calling the component.
7346+
} // The effect "instance" is a shared object that remains the same for the entire
7347+
// lifetime of an effect. In Rust terms, a RefCell. We use it to store the
7348+
// "destroy" function that is returned from an effect, because that is stateful.
7349+
// The field is `undefined` if the effect is unmounted, or if the effect ran
7350+
// but is not stateful. We don't explicitly track whether the effect is mounted
7351+
// or unmounted because that can be inferred by the hiddenness of the fiber in
7352+
// the tree, i.e. whether there is a hidden Offscreen fiber above it.
7353+
//
7354+
// It's unfortunate that this is stored on a separate object, because it adds
7355+
// more memory per effect instance, but it's conceptually sound. I think there's
7356+
// likely a better data structure we could use for effects; perhaps just one
7357+
// array of effect instances per fiber. But I think this is OK for now despite
7358+
// the additional memory and we can follow up with performance
7359+
// optimizations later.
7360+
// These are set right before calling the component.
73477361

73487362
var renderLanes$1 = NoLanes; // The work-in-progress fiber. I've named it differently to distinguish it from
73497363
// the work-in-progress hook.
@@ -8652,7 +8666,7 @@ function mountSyncExternalStore(subscribe, getSnapshot, getServerSnapshot) {
86528666
pushEffect(
86538667
HasEffect | Passive,
86548668
updateStoreInstance.bind(null, fiber, inst, nextSnapshot, getSnapshot),
8655-
undefined,
8669+
createEffectInstance(),
86568670
null
86578671
);
86588672
return nextSnapshot;
@@ -8707,7 +8721,7 @@ function updateSyncExternalStore(subscribe, getSnapshot, getServerSnapshot) {
87078721
pushEffect(
87088722
HasEffect | Passive,
87098723
updateStoreInstance.bind(null, fiber, inst, nextSnapshot, getSnapshot),
8710-
undefined,
8724+
createEffectInstance(),
87118725
null
87128726
); // Unless we're rendering a blocking lane, schedule a consistency check.
87138727
// Right before committing, we will walk the tree and check if any of the
@@ -8832,11 +8846,11 @@ function rerenderState(initialState) {
88328846
return rerenderReducer(basicStateReducer);
88338847
}
88348848

8835-
function pushEffect(tag, create, destroy, deps) {
8849+
function pushEffect(tag, create, inst, deps) {
88368850
var effect = {
88378851
tag: tag,
88388852
create: create,
8839-
destroy: destroy,
8853+
inst: inst,
88408854
deps: deps,
88418855
// Circular
88428856
next: null
@@ -8863,6 +8877,12 @@ function pushEffect(tag, create, destroy, deps) {
88638877
return effect;
88648878
}
88658879

8880+
function createEffectInstance() {
8881+
return {
8882+
destroy: undefined
8883+
};
8884+
}
8885+
88668886
var stackContainsErrorMessage = null;
88678887

88688888
function getCallerStackFrame() {
@@ -8963,25 +8983,24 @@ function mountEffectImpl(fiberFlags, hookFlags, create, deps) {
89638983
hook.memoizedState = pushEffect(
89648984
HasEffect | hookFlags,
89658985
create,
8966-
undefined,
8986+
createEffectInstance(),
89678987
nextDeps
89688988
);
89698989
}
89708990

89718991
function updateEffectImpl(fiberFlags, hookFlags, create, deps) {
89728992
var hook = updateWorkInProgressHook();
89738993
var nextDeps = deps === undefined ? null : deps;
8974-
var destroy = undefined; // currentHook is null when rerendering after a render phase state update.
8994+
var effect = hook.memoizedState;
8995+
var inst = effect.inst; // currentHook is null when rerendering after a render phase state update.
89758996

89768997
if (currentHook !== null) {
8977-
var prevEffect = currentHook.memoizedState;
8978-
destroy = prevEffect.destroy;
8979-
89808998
if (nextDeps !== null) {
8999+
var prevEffect = currentHook.memoizedState;
89819000
var prevDeps = prevEffect.deps;
89829001

89839002
if (areHookInputsEqual(nextDeps, prevDeps)) {
8984-
hook.memoizedState = pushEffect(hookFlags, create, destroy, nextDeps);
9003+
hook.memoizedState = pushEffect(hookFlags, create, inst, nextDeps);
89859004
return;
89869005
}
89879006
}
@@ -8991,7 +9010,7 @@ function updateEffectImpl(fiberFlags, hookFlags, create, deps) {
89919010
hook.memoizedState = pushEffect(
89929011
HasEffect | hookFlags,
89939012
create,
8994-
destroy,
9013+
inst,
89959014
nextDeps
89969015
);
89979016
}
@@ -19298,10 +19317,12 @@ function commitHookEffectListUnmount(
1929819317
do {
1929919318
if ((effect.tag & flags) === flags) {
1930019319
// Unmount
19301-
var destroy = effect.destroy;
19302-
effect.destroy = undefined;
19320+
var inst = effect.inst;
19321+
var destroy = inst.destroy;
1930319322

1930419323
if (destroy !== undefined) {
19324+
inst.destroy = undefined;
19325+
1930519326
if (enableSchedulingProfiler) {
1930619327
if ((flags & Passive) !== NoFlags) {
1930719328
markComponentPassiveEffectUnmountStarted(finishedWork);
@@ -19365,7 +19386,9 @@ function commitHookEffectListMount(flags, finishedWork) {
1936519386
}
1936619387
}
1936719388

19368-
effect.destroy = create();
19389+
var inst = effect.inst;
19390+
var destroy = create();
19391+
inst.destroy = destroy;
1936919392

1937019393
{
1937119394
if ((flags & Insertion) !== NoFlags) {
@@ -19382,8 +19405,6 @@ function commitHookEffectListMount(flags, finishedWork) {
1938219405
}
1938319406

1938419407
{
19385-
var destroy = effect.destroy;
19386-
1938719408
if (destroy !== undefined && typeof destroy !== "function") {
1938819409
var hookName = void 0;
1938919410

@@ -20767,12 +20788,13 @@ function commitDeletionEffectsOnFiber(
2076720788
var effect = firstEffect;
2076820789

2076920790
do {
20770-
var _effect = effect,
20771-
destroy = _effect.destroy,
20772-
tag = _effect.tag;
20791+
var tag = effect.tag;
20792+
var inst = effect.inst;
20793+
var destroy = inst.destroy;
2077320794

2077420795
if (destroy !== undefined) {
2077520796
if ((tag & Insertion) !== NoFlags) {
20797+
inst.destroy = undefined;
2077620798
safelyCallDestroy(
2077720799
deletedFiber,
2077820800
nearestMountedAncestor,
@@ -20785,13 +20807,15 @@ function commitDeletionEffectsOnFiber(
2078520807

2078620808
if (shouldProfile(deletedFiber)) {
2078720809
startLayoutEffectTimer();
20810+
inst.destroy = undefined;
2078820811
safelyCallDestroy(
2078920812
deletedFiber,
2079020813
nearestMountedAncestor,
2079120814
destroy
2079220815
);
2079320816
recordLayoutEffectDuration(deletedFiber);
2079420817
} else {
20818+
inst.destroy = undefined;
2079520819
safelyCallDestroy(
2079620820
deletedFiber,
2079720821
nearestMountedAncestor,

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

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
6969
return self;
7070
}
7171

72-
var ReactVersion = "18.3.0-www-modern-89053274";
72+
var ReactVersion = "18.3.0-www-modern-01966321";
7373

7474
var LegacyRoot = 0;
7575
var ConcurrentRoot = 1;
@@ -7099,7 +7099,21 @@ var didWarnAboutUseWrappedInTryCatch;
70997099
{
71007100
didWarnAboutMismatchedHooksForComponent = new Set();
71017101
didWarnAboutUseWrappedInTryCatch = new Set();
7102-
} // These are set right before calling the component.
7102+
} // The effect "instance" is a shared object that remains the same for the entire
7103+
// lifetime of an effect. In Rust terms, a RefCell. We use it to store the
7104+
// "destroy" function that is returned from an effect, because that is stateful.
7105+
// The field is `undefined` if the effect is unmounted, or if the effect ran
7106+
// but is not stateful. We don't explicitly track whether the effect is mounted
7107+
// or unmounted because that can be inferred by the hiddenness of the fiber in
7108+
// the tree, i.e. whether there is a hidden Offscreen fiber above it.
7109+
//
7110+
// It's unfortunate that this is stored on a separate object, because it adds
7111+
// more memory per effect instance, but it's conceptually sound. I think there's
7112+
// likely a better data structure we could use for effects; perhaps just one
7113+
// array of effect instances per fiber. But I think this is OK for now despite
7114+
// the additional memory and we can follow up with performance
7115+
// optimizations later.
7116+
// These are set right before calling the component.
71037117

71047118
var renderLanes$1 = NoLanes; // The work-in-progress fiber. I've named it differently to distinguish it from
71057119
// the work-in-progress hook.
@@ -8408,7 +8422,7 @@ function mountSyncExternalStore(subscribe, getSnapshot, getServerSnapshot) {
84088422
pushEffect(
84098423
HasEffect | Passive,
84108424
updateStoreInstance.bind(null, fiber, inst, nextSnapshot, getSnapshot),
8411-
undefined,
8425+
createEffectInstance(),
84128426
null
84138427
);
84148428
return nextSnapshot;
@@ -8463,7 +8477,7 @@ function updateSyncExternalStore(subscribe, getSnapshot, getServerSnapshot) {
84638477
pushEffect(
84648478
HasEffect | Passive,
84658479
updateStoreInstance.bind(null, fiber, inst, nextSnapshot, getSnapshot),
8466-
undefined,
8480+
createEffectInstance(),
84678481
null
84688482
); // Unless we're rendering a blocking lane, schedule a consistency check.
84698483
// Right before committing, we will walk the tree and check if any of the
@@ -8588,11 +8602,11 @@ function rerenderState(initialState) {
85888602
return rerenderReducer(basicStateReducer);
85898603
}
85908604

8591-
function pushEffect(tag, create, destroy, deps) {
8605+
function pushEffect(tag, create, inst, deps) {
85928606
var effect = {
85938607
tag: tag,
85948608
create: create,
8595-
destroy: destroy,
8609+
inst: inst,
85968610
deps: deps,
85978611
// Circular
85988612
next: null
@@ -8619,6 +8633,12 @@ function pushEffect(tag, create, destroy, deps) {
86198633
return effect;
86208634
}
86218635

8636+
function createEffectInstance() {
8637+
return {
8638+
destroy: undefined
8639+
};
8640+
}
8641+
86228642
var stackContainsErrorMessage = null;
86238643

86248644
function getCallerStackFrame() {
@@ -8719,25 +8739,24 @@ function mountEffectImpl(fiberFlags, hookFlags, create, deps) {
87198739
hook.memoizedState = pushEffect(
87208740
HasEffect | hookFlags,
87218741
create,
8722-
undefined,
8742+
createEffectInstance(),
87238743
nextDeps
87248744
);
87258745
}
87268746

87278747
function updateEffectImpl(fiberFlags, hookFlags, create, deps) {
87288748
var hook = updateWorkInProgressHook();
87298749
var nextDeps = deps === undefined ? null : deps;
8730-
var destroy = undefined; // currentHook is null when rerendering after a render phase state update.
8750+
var effect = hook.memoizedState;
8751+
var inst = effect.inst; // currentHook is null when rerendering after a render phase state update.
87318752

87328753
if (currentHook !== null) {
8733-
var prevEffect = currentHook.memoizedState;
8734-
destroy = prevEffect.destroy;
8735-
87368754
if (nextDeps !== null) {
8755+
var prevEffect = currentHook.memoizedState;
87378756
var prevDeps = prevEffect.deps;
87388757

87398758
if (areHookInputsEqual(nextDeps, prevDeps)) {
8740-
hook.memoizedState = pushEffect(hookFlags, create, destroy, nextDeps);
8759+
hook.memoizedState = pushEffect(hookFlags, create, inst, nextDeps);
87418760
return;
87428761
}
87438762
}
@@ -8747,7 +8766,7 @@ function updateEffectImpl(fiberFlags, hookFlags, create, deps) {
87478766
hook.memoizedState = pushEffect(
87488767
HasEffect | hookFlags,
87498768
create,
8750-
destroy,
8769+
inst,
87518770
nextDeps
87528771
);
87538772
}
@@ -18963,10 +18982,12 @@ function commitHookEffectListUnmount(
1896318982
do {
1896418983
if ((effect.tag & flags) === flags) {
1896518984
// Unmount
18966-
var destroy = effect.destroy;
18967-
effect.destroy = undefined;
18985+
var inst = effect.inst;
18986+
var destroy = inst.destroy;
1896818987

1896918988
if (destroy !== undefined) {
18989+
inst.destroy = undefined;
18990+
1897018991
if (enableSchedulingProfiler) {
1897118992
if ((flags & Passive) !== NoFlags) {
1897218993
markComponentPassiveEffectUnmountStarted(finishedWork);
@@ -19030,7 +19051,9 @@ function commitHookEffectListMount(flags, finishedWork) {
1903019051
}
1903119052
}
1903219053

19033-
effect.destroy = create();
19054+
var inst = effect.inst;
19055+
var destroy = create();
19056+
inst.destroy = destroy;
1903419057

1903519058
{
1903619059
if ((flags & Insertion) !== NoFlags) {
@@ -19047,8 +19070,6 @@ function commitHookEffectListMount(flags, finishedWork) {
1904719070
}
1904819071

1904919072
{
19050-
var destroy = effect.destroy;
19051-
1905219073
if (destroy !== undefined && typeof destroy !== "function") {
1905319074
var hookName = void 0;
1905419075

@@ -20432,12 +20453,13 @@ function commitDeletionEffectsOnFiber(
2043220453
var effect = firstEffect;
2043320454

2043420455
do {
20435-
var _effect = effect,
20436-
destroy = _effect.destroy,
20437-
tag = _effect.tag;
20456+
var tag = effect.tag;
20457+
var inst = effect.inst;
20458+
var destroy = inst.destroy;
2043820459

2043920460
if (destroy !== undefined) {
2044020461
if ((tag & Insertion) !== NoFlags) {
20462+
inst.destroy = undefined;
2044120463
safelyCallDestroy(
2044220464
deletedFiber,
2044320465
nearestMountedAncestor,
@@ -20450,13 +20472,15 @@ function commitDeletionEffectsOnFiber(
2045020472

2045120473
if (shouldProfile(deletedFiber)) {
2045220474
startLayoutEffectTimer();
20475+
inst.destroy = undefined;
2045320476
safelyCallDestroy(
2045420477
deletedFiber,
2045520478
nearestMountedAncestor,
2045620479
destroy
2045720480
);
2045820481
recordLayoutEffectDuration(deletedFiber);
2045920482
} else {
20483+
inst.destroy = undefined;
2046020484
safelyCallDestroy(
2046120485
deletedFiber,
2046220486
nearestMountedAncestor,

0 commit comments

Comments
 (0)