Skip to content

Commit

Permalink
Warn if optimistic state is updated outside of a transition (#27454)
Browse files Browse the repository at this point in the history
### Based on #27453

If optimistic state is updated, and there's no startTransition on the
stack, there are two likely scenarios.

One possibility is that the optimistic update is triggered by a regular
event handler (e.g. `onSubmit`) instead of an action. This is a mistake
and we will warn.

The other possibility is the optimistic update is inside an async
action, but after an `await`. In this case, we can make it "just work"
by associating the optimistic update with the pending async action.

Technically it's possible that the optimistic update is unrelated to the
pending action, but we don't have a way of knowing this for sure because
browsers currently do not provide a way to track async scope. (The
AsyncContext proposal, if it lands, will solve this in the future.)
However, this is no different than the problem of unrelated transitions
being grouped together — it's not wrong per se, but it's not ideal.

Once AsyncContext starts landing in browsers, we will provide better
warnings in development for these cases.

DiffTrain build for [88d56b8](88d56b8)
  • Loading branch information
acdlite committed Oct 4, 2023
1 parent c4f430b commit 3dcc034
Show file tree
Hide file tree
Showing 24 changed files with 300 additions and 103 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
85c2b519b54269811002d26f4f711809ef68f123
88d56b8e818d0c48eb6642303169c1fadeb99d59
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-d32d685e";
var ReactVersion = "18.3.0-www-classic-f9aace21";

// ATTENTION
// When adding new symbols to this file,
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -623,4 +623,4 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-www-classic-29f3c7ed";
exports.version = "18.3.0-www-classic-03d59dfb";
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-d97fc327";
exports.version = "18.3.0-www-modern-12525118";
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-7e2c0d38";
exports.version = "18.3.0-www-modern-fa7e58e7";

/* global __REACT_DEVTOOLS_GLOBAL_HOOK__ */
if (
Expand Down
42 changes: 36 additions & 6 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-86227a5b";
var ReactVersion = "18.3.0-www-classic-dbe5a0bf";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down Expand Up @@ -9822,6 +9822,7 @@ function startTransition(
higherEventPriority(previousPriority, ContinuousEventPriority)
);
var prevTransition = ReactCurrentBatchConfig$2.transition;
var currentTransition = {};

if (enableAsyncActions) {
// We don't really need to use an optimistic update here, because we
Expand All @@ -9830,14 +9831,14 @@ function startTransition(
// optimistic update anyway to make it less likely the behavior accidentally
// diverges; for example, both an optimistic update and this one should
// share the same lane.
ReactCurrentBatchConfig$2.transition = currentTransition;
dispatchOptimisticSetState(fiber, false, queue, pendingState);
} else {
ReactCurrentBatchConfig$2.transition = null;
dispatchSetState(fiber, queue, pendingState);
ReactCurrentBatchConfig$2.transition = currentTransition;
}

var currentTransition = (ReactCurrentBatchConfig$2.transition = {});

if (enableTransitionTracing) {
if (options !== undefined && options.name !== undefined) {
ReactCurrentBatchConfig$2.transition.name = options.name;
Expand Down Expand Up @@ -10173,14 +10174,43 @@ function dispatchSetState(fiber, queue, action) {
}

function dispatchOptimisticSetState(fiber, throwIfDuringRender, queue, action) {
{
if (ReactCurrentBatchConfig$2.transition === null) {
// An optimistic update occurred, but startTransition is not on the stack.
// There are two likely scenarios.
// One possibility is that the optimistic update is triggered by a regular
// event handler (e.g. `onSubmit`) instead of an action. This is a mistake
// and we will warn.
// The other possibility is the optimistic update is inside an async
// action, but after an `await`. In this case, we can make it "just work"
// by associating the optimistic update with the pending async action.
// Technically it's possible that the optimistic update is unrelated to
// the pending action, but we don't have a way of knowing this for sure
// because browsers currently do not provide a way to track async scope.
// (The AsyncContext proposal, if it lands, will solve this in the
// future.) However, this is no different than the problem of unrelated
// transitions being grouped together — it's not wrong per se, but it's
// not ideal.
// Once AsyncContext starts landing in browsers, we will provide better
// warnings in development for these cases.
if (peekEntangledActionLane() !== NoLane);
else {
// There's no pending async action. The most likely cause is that we're
// inside a regular event handler (e.g. onSubmit) instead of an action.
error(
"An optimistic state update occurred outside a transition or " +
"action. To fix, move the update to an action, or wrap " +
"with startTransition."
);
}
}
}

var update = {
// An optimistic update commits synchronously.
lane: SyncLane,
// After committing, the optimistic update is "reverted" using the same
// lane as the transition it's associated with.
//
// TODO: Warn if there's no transition/action associated with this
// optimistic update.
revertLane: requestTransitionLane(),
action: action,
hasEagerState: false,
Expand Down
42 changes: 36 additions & 6 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-cb89a149";
var ReactVersion = "18.3.0-www-modern-73c8edac";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down Expand Up @@ -9578,6 +9578,7 @@ function startTransition(
higherEventPriority(previousPriority, ContinuousEventPriority)
);
var prevTransition = ReactCurrentBatchConfig$2.transition;
var currentTransition = {};

if (enableAsyncActions) {
// We don't really need to use an optimistic update here, because we
Expand All @@ -9586,14 +9587,14 @@ function startTransition(
// optimistic update anyway to make it less likely the behavior accidentally
// diverges; for example, both an optimistic update and this one should
// share the same lane.
ReactCurrentBatchConfig$2.transition = currentTransition;
dispatchOptimisticSetState(fiber, false, queue, pendingState);
} else {
ReactCurrentBatchConfig$2.transition = null;
dispatchSetState(fiber, queue, pendingState);
ReactCurrentBatchConfig$2.transition = currentTransition;
}

var currentTransition = (ReactCurrentBatchConfig$2.transition = {});

if (enableTransitionTracing) {
if (options !== undefined && options.name !== undefined) {
ReactCurrentBatchConfig$2.transition.name = options.name;
Expand Down Expand Up @@ -9929,14 +9930,43 @@ function dispatchSetState(fiber, queue, action) {
}

function dispatchOptimisticSetState(fiber, throwIfDuringRender, queue, action) {
{
if (ReactCurrentBatchConfig$2.transition === null) {
// An optimistic update occurred, but startTransition is not on the stack.
// There are two likely scenarios.
// One possibility is that the optimistic update is triggered by a regular
// event handler (e.g. `onSubmit`) instead of an action. This is a mistake
// and we will warn.
// The other possibility is the optimistic update is inside an async
// action, but after an `await`. In this case, we can make it "just work"
// by associating the optimistic update with the pending async action.
// Technically it's possible that the optimistic update is unrelated to
// the pending action, but we don't have a way of knowing this for sure
// because browsers currently do not provide a way to track async scope.
// (The AsyncContext proposal, if it lands, will solve this in the
// future.) However, this is no different than the problem of unrelated
// transitions being grouped together — it's not wrong per se, but it's
// not ideal.
// Once AsyncContext starts landing in browsers, we will provide better
// warnings in development for these cases.
if (peekEntangledActionLane() !== NoLane);
else {
// There's no pending async action. The most likely cause is that we're
// inside a regular event handler (e.g. onSubmit) instead of an action.
error(
"An optimistic state update occurred outside a transition or " +
"action. To fix, move the update to an action, or wrap " +
"with startTransition."
);
}
}
}

var update = {
// An optimistic update commits synchronously.
lane: SyncLane,
// After committing, the optimistic update is "reverted" using the same
// lane as the transition it's associated with.
//
// TODO: Warn if there's no transition/action associated with this
// optimistic update.
revertLane: requestTransitionLane(),
action: action,
hasEagerState: false,
Expand Down
14 changes: 8 additions & 6 deletions compiled/facebook-www/ReactART-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -3158,12 +3158,14 @@ function startTransition(
var previousPriority = currentUpdatePriority;
currentUpdatePriority =
0 !== previousPriority && 8 > previousPriority ? previousPriority : 8;
var prevTransition = ReactCurrentBatchConfig$2.transition;
var prevTransition = ReactCurrentBatchConfig$2.transition,
currentTransition = {};
enableAsyncActions
? dispatchOptimisticSetState(fiber, !1, queue, pendingState)
? ((ReactCurrentBatchConfig$2.transition = currentTransition),
dispatchOptimisticSetState(fiber, !1, queue, pendingState))
: ((ReactCurrentBatchConfig$2.transition = null),
dispatchSetState(fiber, queue, pendingState));
ReactCurrentBatchConfig$2.transition = {};
dispatchSetState(fiber, queue, pendingState),
(ReactCurrentBatchConfig$2.transition = currentTransition));
enableTransitionTracing &&
void 0 !== options &&
void 0 !== options.name &&
Expand Down Expand Up @@ -10107,7 +10109,7 @@ var slice = Array.prototype.slice,
return null;
},
bundleType: 0,
version: "18.3.0-www-classic-8867a970",
version: "18.3.0-www-classic-bf99e5b1",
rendererPackageName: "react-art"
};
var internals$jscomp$inline_1304 = {
Expand Down Expand Up @@ -10138,7 +10140,7 @@ var internals$jscomp$inline_1304 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-www-classic-8867a970"
reconcilerVersion: "18.3.0-www-classic-bf99e5b1"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1305 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
14 changes: 8 additions & 6 deletions compiled/facebook-www/ReactART-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -2964,12 +2964,14 @@ function startTransition(
var previousPriority = currentUpdatePriority;
currentUpdatePriority =
0 !== previousPriority && 8 > previousPriority ? previousPriority : 8;
var prevTransition = ReactCurrentBatchConfig$2.transition;
var prevTransition = ReactCurrentBatchConfig$2.transition,
currentTransition = {};
enableAsyncActions
? dispatchOptimisticSetState(fiber, !1, queue, pendingState)
? ((ReactCurrentBatchConfig$2.transition = currentTransition),
dispatchOptimisticSetState(fiber, !1, queue, pendingState))
: ((ReactCurrentBatchConfig$2.transition = null),
dispatchSetState(fiber, queue, pendingState));
ReactCurrentBatchConfig$2.transition = {};
dispatchSetState(fiber, queue, pendingState),
(ReactCurrentBatchConfig$2.transition = currentTransition));
enableTransitionTracing &&
void 0 !== options &&
void 0 !== options.name &&
Expand Down Expand Up @@ -9772,7 +9774,7 @@ var slice = Array.prototype.slice,
return null;
},
bundleType: 0,
version: "18.3.0-www-modern-46965381",
version: "18.3.0-www-modern-e0184800",
rendererPackageName: "react-art"
};
var internals$jscomp$inline_1284 = {
Expand Down Expand Up @@ -9803,7 +9805,7 @@ var internals$jscomp$inline_1284 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-www-modern-46965381"
reconcilerVersion: "18.3.0-www-modern-e0184800"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1285 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
42 changes: 36 additions & 6 deletions compiled/facebook-www/ReactDOM-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -14369,6 +14369,7 @@ function startTransition(
higherEventPriority(previousPriority, ContinuousEventPriority)
);
var prevTransition = ReactCurrentBatchConfig$3.transition;
var currentTransition = {};

if (enableAsyncActions) {
// We don't really need to use an optimistic update here, because we
Expand All @@ -14377,14 +14378,14 @@ function startTransition(
// optimistic update anyway to make it less likely the behavior accidentally
// diverges; for example, both an optimistic update and this one should
// share the same lane.
ReactCurrentBatchConfig$3.transition = currentTransition;
dispatchOptimisticSetState(fiber, false, queue, pendingState);
} else {
ReactCurrentBatchConfig$3.transition = null;
dispatchSetState(fiber, queue, pendingState);
ReactCurrentBatchConfig$3.transition = currentTransition;
}

var currentTransition = (ReactCurrentBatchConfig$3.transition = {});

if (enableTransitionTracing) {
if (options !== undefined && options.name !== undefined) {
ReactCurrentBatchConfig$3.transition.name = options.name;
Expand Down Expand Up @@ -14734,14 +14735,43 @@ function dispatchSetState(fiber, queue, action) {
}

function dispatchOptimisticSetState(fiber, throwIfDuringRender, queue, action) {
{
if (ReactCurrentBatchConfig$3.transition === null) {
// An optimistic update occurred, but startTransition is not on the stack.
// There are two likely scenarios.
// One possibility is that the optimistic update is triggered by a regular
// event handler (e.g. `onSubmit`) instead of an action. This is a mistake
// and we will warn.
// The other possibility is the optimistic update is inside an async
// action, but after an `await`. In this case, we can make it "just work"
// by associating the optimistic update with the pending async action.
// Technically it's possible that the optimistic update is unrelated to
// the pending action, but we don't have a way of knowing this for sure
// because browsers currently do not provide a way to track async scope.
// (The AsyncContext proposal, if it lands, will solve this in the
// future.) However, this is no different than the problem of unrelated
// transitions being grouped together — it's not wrong per se, but it's
// not ideal.
// Once AsyncContext starts landing in browsers, we will provide better
// warnings in development for these cases.
if (peekEntangledActionLane() !== NoLane);
else {
// There's no pending async action. The most likely cause is that we're
// inside a regular event handler (e.g. onSubmit) instead of an action.
error(
"An optimistic state update occurred outside a transition or " +
"action. To fix, move the update to an action, or wrap " +
"with startTransition."
);
}
}
}

var update = {
// An optimistic update commits synchronously.
lane: SyncLane,
// After committing, the optimistic update is "reverted" using the same
// lane as the transition it's associated with.
//
// TODO: Warn if there's no transition/action associated with this
// optimistic update.
revertLane: requestTransitionLane(),
action: action,
hasEagerState: false,
Expand Down Expand Up @@ -33976,7 +34006,7 @@ function createFiberRoot(
return root;
}

var ReactVersion = "18.3.0-www-classic-43da500c";
var ReactVersion = "18.3.0-www-classic-07a731ef";

function createPortal$1(
children,
Expand Down
Loading

0 comments on commit 3dcc034

Please sign in to comment.