Skip to content

Commit

Permalink
Fix: Optimistic update does not get reset (#27453)
Browse files Browse the repository at this point in the history
I found a bug where if an optimistic update causes a component to
rerender, and there are no other state updates during that render, React
bails out without applying the update.

Whenever a hook detects a change, we must mark the component as dirty to
prevent a bailout. We check for changes by comparing the new state to
`hook.memoizedState`. However, when implementing optimistic state
rebasing, I incorrectly reset `hook.memoizedState` to the incoming base
state, even though I only needed to reset `hook.baseState`. This was
just a mistake on my part.

This wasn't caught by the existing tests because usually when the
optimistic state changes, there's also some other state that marks the
component as dirty in the same render.

I fixed the bug and added a regression test.

DiffTrain build for [85c2b51](85c2b51)
  • Loading branch information
acdlite committed Oct 3, 2023
1 parent 3d74031 commit c4f430b
Show file tree
Hide file tree
Showing 20 changed files with 86 additions and 86 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
db69f95e4876ec3c24117f58d55cbb4f315b9fa7
85c2b519b54269811002d26f4f711809ef68f123
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ if (
}
"use strict";

var ReactVersion = "18.3.0-www-classic-136a3d2d";
var ReactVersion = "18.3.0-www-classic-d32d685e";

// ATTENTION
// When adding new symbols to this file,
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -615,4 +615,4 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-www-modern-939a7938";
exports.version = "18.3.0-www-modern-d97fc327";
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-profiling.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-www-modern-de10697a";
exports.version = "18.3.0-www-modern-7e2c0d38";

/* global __REACT_DEVTOOLS_GLOBAL_HOOK__ */
if (
Expand Down
14 changes: 7 additions & 7 deletions compiled/facebook-www/ReactART-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
return self;
}

var ReactVersion = "18.3.0-www-classic-6b865fa6";
var ReactVersion = "18.3.0-www-classic-86227a5b";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down Expand Up @@ -9296,9 +9296,9 @@ function updateOptimisticImpl(hook, current, passthrough, reducer) {
// as an argument. It's called a passthrough because if there are no pending
// updates, it will be returned as-is.
//
// Reset the base state and memoized state to the passthrough. Future
// updates will be applied on top of this.
hook.baseState = hook.memoizedState = passthrough; // If a reducer is not provided, default to the same one used by useState.
// Reset the base state to the passthrough. Future updates will be applied
// on top of this.
hook.baseState = passthrough; // If a reducer is not provided, default to the same one used by useState.

var resolvedReducer =
typeof reducer === "function" ? reducer : basicStateReducer;
Expand All @@ -9319,10 +9319,10 @@ function rerenderOptimistic(passthrough, reducer) {
// This is an update. Process the update queue.
return updateOptimisticImpl(hook, currentHook, passthrough, reducer);
} // This is a mount. No updates to process.
// Reset the base state and memoized state to the passthrough. Future
// updates will be applied on top of this.
// Reset the base state to the passthrough. Future updates will be applied
// on top of this.

hook.baseState = hook.memoizedState = passthrough;
hook.baseState = passthrough;
var dispatch = hook.queue.dispatch;
return [passthrough, dispatch];
} // useFormState actions run sequentially, because each action receives the
Expand Down
14 changes: 7 additions & 7 deletions compiled/facebook-www/ReactART-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
return self;
}

var ReactVersion = "18.3.0-www-modern-5845cabd";
var ReactVersion = "18.3.0-www-modern-cb89a149";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down Expand Up @@ -9052,9 +9052,9 @@ function updateOptimisticImpl(hook, current, passthrough, reducer) {
// as an argument. It's called a passthrough because if there are no pending
// updates, it will be returned as-is.
//
// Reset the base state and memoized state to the passthrough. Future
// updates will be applied on top of this.
hook.baseState = hook.memoizedState = passthrough; // If a reducer is not provided, default to the same one used by useState.
// Reset the base state to the passthrough. Future updates will be applied
// on top of this.
hook.baseState = passthrough; // If a reducer is not provided, default to the same one used by useState.

var resolvedReducer =
typeof reducer === "function" ? reducer : basicStateReducer;
Expand All @@ -9075,10 +9075,10 @@ function rerenderOptimistic(passthrough, reducer) {
// This is an update. Process the update queue.
return updateOptimisticImpl(hook, currentHook, passthrough, reducer);
} // This is a mount. No updates to process.
// Reset the base state and memoized state to the passthrough. Future
// updates will be applied on top of this.
// Reset the base state to the passthrough. Future updates will be applied
// on top of this.

hook.baseState = hook.memoizedState = passthrough;
hook.baseState = passthrough;
var dispatch = hook.queue.dispatch;
return [passthrough, dispatch];
} // useFormState actions run sequentially, because each action receives the
Expand Down
8 changes: 4 additions & 4 deletions compiled/facebook-www/ReactART-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -3003,7 +3003,7 @@ function updateOptimistic(passthrough, reducer) {
return updateOptimisticImpl(hook, currentHook, passthrough, reducer);
}
function updateOptimisticImpl(hook, current, passthrough, reducer) {
hook.baseState = hook.memoizedState = passthrough;
hook.baseState = passthrough;
return updateReducerImpl(
hook,
currentHook,
Expand All @@ -3014,7 +3014,7 @@ function rerenderOptimistic(passthrough, reducer) {
var hook = updateWorkInProgressHook();
if (null !== currentHook)
return updateOptimisticImpl(hook, currentHook, passthrough, reducer);
hook.baseState = hook.memoizedState = passthrough;
hook.baseState = passthrough;
return [passthrough, hook.queue.dispatch];
}
function pushEffect(tag, create, inst, deps) {
Expand Down Expand Up @@ -10107,7 +10107,7 @@ var slice = Array.prototype.slice,
return null;
},
bundleType: 0,
version: "18.3.0-www-classic-2b1e1230",
version: "18.3.0-www-classic-8867a970",
rendererPackageName: "react-art"
};
var internals$jscomp$inline_1304 = {
Expand Down Expand Up @@ -10138,7 +10138,7 @@ var internals$jscomp$inline_1304 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-www-classic-2b1e1230"
reconcilerVersion: "18.3.0-www-classic-8867a970"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1305 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
8 changes: 4 additions & 4 deletions compiled/facebook-www/ReactART-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -2809,7 +2809,7 @@ function updateOptimistic(passthrough, reducer) {
return updateOptimisticImpl(hook, currentHook, passthrough, reducer);
}
function updateOptimisticImpl(hook, current, passthrough, reducer) {
hook.baseState = hook.memoizedState = passthrough;
hook.baseState = passthrough;
return updateReducerImpl(
hook,
currentHook,
Expand All @@ -2820,7 +2820,7 @@ function rerenderOptimistic(passthrough, reducer) {
var hook = updateWorkInProgressHook();
if (null !== currentHook)
return updateOptimisticImpl(hook, currentHook, passthrough, reducer);
hook.baseState = hook.memoizedState = passthrough;
hook.baseState = passthrough;
return [passthrough, hook.queue.dispatch];
}
function pushEffect(tag, create, inst, deps) {
Expand Down Expand Up @@ -9772,7 +9772,7 @@ var slice = Array.prototype.slice,
return null;
},
bundleType: 0,
version: "18.3.0-www-modern-d368bffd",
version: "18.3.0-www-modern-46965381",
rendererPackageName: "react-art"
};
var internals$jscomp$inline_1284 = {
Expand Down Expand Up @@ -9803,7 +9803,7 @@ var internals$jscomp$inline_1284 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-www-modern-d368bffd"
reconcilerVersion: "18.3.0-www-modern-46965381"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1285 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
14 changes: 7 additions & 7 deletions compiled/facebook-www/ReactDOM-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -13843,9 +13843,9 @@ function updateOptimisticImpl(hook, current, passthrough, reducer) {
// as an argument. It's called a passthrough because if there are no pending
// updates, it will be returned as-is.
//
// Reset the base state and memoized state to the passthrough. Future
// updates will be applied on top of this.
hook.baseState = hook.memoizedState = passthrough; // If a reducer is not provided, default to the same one used by useState.
// Reset the base state to the passthrough. Future updates will be applied
// on top of this.
hook.baseState = passthrough; // If a reducer is not provided, default to the same one used by useState.

var resolvedReducer =
typeof reducer === "function" ? reducer : basicStateReducer;
Expand All @@ -13866,10 +13866,10 @@ function rerenderOptimistic(passthrough, reducer) {
// This is an update. Process the update queue.
return updateOptimisticImpl(hook, currentHook, passthrough, reducer);
} // This is a mount. No updates to process.
// Reset the base state and memoized state to the passthrough. Future
// updates will be applied on top of this.
// Reset the base state to the passthrough. Future updates will be applied
// on top of this.

hook.baseState = hook.memoizedState = passthrough;
hook.baseState = passthrough;
var dispatch = hook.queue.dispatch;
return [passthrough, dispatch];
} // useFormState actions run sequentially, because each action receives the
Expand Down Expand Up @@ -33976,7 +33976,7 @@ function createFiberRoot(
return root;
}

var ReactVersion = "18.3.0-www-classic-85c09bff";
var ReactVersion = "18.3.0-www-classic-43da500c";

function createPortal$1(
children,
Expand Down
14 changes: 7 additions & 7 deletions compiled/facebook-www/ReactDOM-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -13784,9 +13784,9 @@ function updateOptimisticImpl(hook, current, passthrough, reducer) {
// as an argument. It's called a passthrough because if there are no pending
// updates, it will be returned as-is.
//
// Reset the base state and memoized state to the passthrough. Future
// updates will be applied on top of this.
hook.baseState = hook.memoizedState = passthrough; // If a reducer is not provided, default to the same one used by useState.
// Reset the base state to the passthrough. Future updates will be applied
// on top of this.
hook.baseState = passthrough; // If a reducer is not provided, default to the same one used by useState.

var resolvedReducer =
typeof reducer === "function" ? reducer : basicStateReducer;
Expand All @@ -13807,10 +13807,10 @@ function rerenderOptimistic(passthrough, reducer) {
// This is an update. Process the update queue.
return updateOptimisticImpl(hook, currentHook, passthrough, reducer);
} // This is a mount. No updates to process.
// Reset the base state and memoized state to the passthrough. Future
// updates will be applied on top of this.
// Reset the base state to the passthrough. Future updates will be applied
// on top of this.

hook.baseState = hook.memoizedState = passthrough;
hook.baseState = passthrough;
var dispatch = hook.queue.dispatch;
return [passthrough, dispatch];
} // useFormState actions run sequentially, because each action receives the
Expand Down Expand Up @@ -33821,7 +33821,7 @@ function createFiberRoot(
return root;
}

var ReactVersion = "18.3.0-www-modern-c7df3b44";
var ReactVersion = "18.3.0-www-modern-a850f7df";

function createPortal$1(
children,
Expand Down
10 changes: 5 additions & 5 deletions compiled/facebook-www/ReactDOM-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -3743,7 +3743,7 @@ function updateOptimistic(passthrough, reducer) {
return updateOptimisticImpl(hook, currentHook, passthrough, reducer);
}
function updateOptimisticImpl(hook, current, passthrough, reducer) {
hook.baseState = hook.memoizedState = passthrough;
hook.baseState = passthrough;
return updateReducerImpl(
hook,
currentHook,
Expand All @@ -3754,7 +3754,7 @@ function rerenderOptimistic(passthrough, reducer) {
var hook = updateWorkInProgressHook();
if (null !== currentHook)
return updateOptimisticImpl(hook, currentHook, passthrough, reducer);
hook.baseState = hook.memoizedState = passthrough;
hook.baseState = passthrough;
return [passthrough, hook.queue.dispatch];
}
function pushEffect(tag, create, inst, deps) {
Expand Down Expand Up @@ -16375,7 +16375,7 @@ Internals.Events = [
var devToolsConfig$jscomp$inline_1779 = {
findFiberByHostInstance: getClosestInstanceFromNode,
bundleType: 0,
version: "18.3.0-www-classic-a202a369",
version: "18.3.0-www-classic-649be7ad",
rendererPackageName: "react-dom"
};
var internals$jscomp$inline_2123 = {
Expand Down Expand Up @@ -16405,7 +16405,7 @@ var internals$jscomp$inline_2123 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-www-classic-a202a369"
reconcilerVersion: "18.3.0-www-classic-649be7ad"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_2124 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down Expand Up @@ -16742,4 +16742,4 @@ exports.unstable_renderSubtreeIntoContainer = function (
);
};
exports.unstable_runWithPriority = runWithPriority;
exports.version = "18.3.0-www-classic-a202a369";
exports.version = "18.3.0-www-classic-649be7ad";
10 changes: 5 additions & 5 deletions compiled/facebook-www/ReactDOM-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -3636,7 +3636,7 @@ function updateOptimistic(passthrough, reducer) {
return updateOptimisticImpl(hook, currentHook, passthrough, reducer);
}
function updateOptimisticImpl(hook, current, passthrough, reducer) {
hook.baseState = hook.memoizedState = passthrough;
hook.baseState = passthrough;
return updateReducerImpl(
hook,
currentHook,
Expand All @@ -3647,7 +3647,7 @@ function rerenderOptimistic(passthrough, reducer) {
var hook = updateWorkInProgressHook();
if (null !== currentHook)
return updateOptimisticImpl(hook, currentHook, passthrough, reducer);
hook.baseState = hook.memoizedState = passthrough;
hook.baseState = passthrough;
return [passthrough, hook.queue.dispatch];
}
function pushEffect(tag, create, inst, deps) {
Expand Down Expand Up @@ -15897,7 +15897,7 @@ Internals.Events = [
var devToolsConfig$jscomp$inline_1738 = {
findFiberByHostInstance: getClosestInstanceFromNode,
bundleType: 0,
version: "18.3.0-www-modern-7e008df6",
version: "18.3.0-www-modern-ea5134e6",
rendererPackageName: "react-dom"
};
var internals$jscomp$inline_2087 = {
Expand Down Expand Up @@ -15928,7 +15928,7 @@ var internals$jscomp$inline_2087 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-www-modern-7e008df6"
reconcilerVersion: "18.3.0-www-modern-ea5134e6"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_2088 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down Expand Up @@ -16193,4 +16193,4 @@ exports.unstable_createEventHandle = function (type, options) {
return eventHandle;
};
exports.unstable_runWithPriority = runWithPriority;
exports.version = "18.3.0-www-modern-7e008df6";
exports.version = "18.3.0-www-modern-ea5134e6";
10 changes: 5 additions & 5 deletions compiled/facebook-www/ReactDOM-profiling.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -3889,7 +3889,7 @@ function updateOptimistic(passthrough, reducer) {
return updateOptimisticImpl(hook, currentHook, passthrough, reducer);
}
function updateOptimisticImpl(hook, current, passthrough, reducer) {
hook.baseState = hook.memoizedState = passthrough;
hook.baseState = passthrough;
return updateReducerImpl(
hook,
currentHook,
Expand All @@ -3900,7 +3900,7 @@ function rerenderOptimistic(passthrough, reducer) {
var hook = updateWorkInProgressHook();
if (null !== currentHook)
return updateOptimisticImpl(hook, currentHook, passthrough, reducer);
hook.baseState = hook.memoizedState = passthrough;
hook.baseState = passthrough;
return [passthrough, hook.queue.dispatch];
}
function pushEffect(tag, create, inst, deps) {
Expand Down Expand Up @@ -17150,7 +17150,7 @@ Internals.Events = [
var devToolsConfig$jscomp$inline_1864 = {
findFiberByHostInstance: getClosestInstanceFromNode,
bundleType: 0,
version: "18.3.0-www-classic-bf660d13",
version: "18.3.0-www-classic-ea6b16b5",
rendererPackageName: "react-dom"
};
(function (internals) {
Expand Down Expand Up @@ -17194,7 +17194,7 @@ var devToolsConfig$jscomp$inline_1864 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-www-classic-bf660d13"
reconcilerVersion: "18.3.0-www-classic-ea6b16b5"
});
assign(Internals, {
ReactBrowserEventEmitter: {
Expand Down Expand Up @@ -17518,7 +17518,7 @@ exports.unstable_renderSubtreeIntoContainer = function (
);
};
exports.unstable_runWithPriority = runWithPriority;
exports.version = "18.3.0-www-classic-bf660d13";
exports.version = "18.3.0-www-classic-ea6b16b5";

/* global __REACT_DEVTOOLS_GLOBAL_HOOK__ */
if (
Expand Down
Loading

0 comments on commit c4f430b

Please sign in to comment.