Skip to content

Commit

Permalink
Fix async batching in React.startTransition (#29226)
Browse files Browse the repository at this point in the history
Note: Despite the similar-sounding description, this fix is unrelated to
the issue where updates that occur after an `await` in an async action
must also be wrapped in their own `startTransition`, due to the absence
of an AsyncContext mechanism in browsers today.

---

Discovered a flaw in the current implementation of the isomorphic
startTransition implementation (React.startTransition), related to async
actions. It only creates an async scope if something calls setState
within the synchronous part of the action (i.e. before the first
`await`). I had thought this was fine because if there's no update
during this part, then there's nothing that needs to be entangled. I
didn't think this through, though — if there are multiple async updates
interleaved throughout the rest of the action, we need the async scope
to have already been created so that _those_ are batched together.

An even easier way to observe this is to schedule an optimistic update
after an `await` — the optimistic update should not be reverted until
the async action is complete.

To implement, during the reconciler's module initialization, we compose
its startTransition implementation with any previous reconciler's
startTransition that was already initialized. Then, the isomorphic
startTransition is the composition of every
reconciler's startTransition.

```js
function startTransition(fn) {
  return startTransitionDOM(() => {
    return startTransitionART(() => {
      return startTransitionThreeFiber(() => {
        // and so on...
        return fn();
      });
    });
  });
}
```

This is basically how flushSync is implemented, too.

DiffTrain build for [ee5c194](ee5c194)
  • Loading branch information
acdlite committed May 23, 2024
1 parent fe342b1 commit 4374cb6
Show file tree
Hide file tree
Showing 27 changed files with 783 additions and 639 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
f55d172bcf921d761733533395b798c5b3665e04
ee5c19493086fdeb32057e16d1e3414370242307
22 changes: 10 additions & 12 deletions compiled/facebook-www/React-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ if (
) {
__REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStart(new Error());
}
var ReactVersion = '19.0.0-www-classic-87c03497';
var ReactVersion = '19.0.0-www-classic-f3d67148';

// Re-export dynamic flags from the www version.
var dynamicFeatureFlags = require('ReactFeatureFlags');
Expand Down Expand Up @@ -564,7 +564,8 @@ function getComponentNameFromType(type) {
var ReactSharedInternals = {
H: null,
A: null,
T: null
T: null,
S: null
};

{
Expand Down Expand Up @@ -2956,13 +2957,8 @@ reportError : function (error) {
};

function startTransition(scope, options) {
var prevTransition = ReactSharedInternals.T; // Each renderer registers a callback to receive the return value of
// the scope function. This is used to implement async actions.

var callbacks = new Set();
var transition = {
_callbacks: callbacks
};
var prevTransition = ReactSharedInternals.T;
var transition = {};
ReactSharedInternals.T = transition;
var currentTransition = ReactSharedInternals.T;

Expand All @@ -2982,11 +2978,13 @@ function startTransition(scope, options) {
{
try {
var returnValue = scope();
var onStartTransitionFinish = ReactSharedInternals.S;

if (onStartTransitionFinish !== null) {
onStartTransitionFinish(transition, returnValue);
}

if (typeof returnValue === 'object' && returnValue !== null && typeof returnValue.then === 'function') {
callbacks.forEach(function (callback) {
return callback(currentTransition, returnValue);
});
returnValue.then(noop, reportGlobalError);
}
} catch (error) {
Expand Down
22 changes: 10 additions & 12 deletions compiled/facebook-www/React-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ if (
) {
__REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStart(new Error());
}
var ReactVersion = '19.0.0-www-modern-005a649e';
var ReactVersion = '19.0.0-www-modern-7638663c';

// Re-export dynamic flags from the www version.
var dynamicFeatureFlags = require('ReactFeatureFlags');
Expand Down Expand Up @@ -564,7 +564,8 @@ function getComponentNameFromType(type) {
var ReactSharedInternals = {
H: null,
A: null,
T: null
T: null,
S: null
};

{
Expand Down Expand Up @@ -2959,13 +2960,8 @@ reportError : function (error) {
};

function startTransition(scope, options) {
var prevTransition = ReactSharedInternals.T; // Each renderer registers a callback to receive the return value of
// the scope function. This is used to implement async actions.

var callbacks = new Set();
var transition = {
_callbacks: callbacks
};
var prevTransition = ReactSharedInternals.T;
var transition = {};
ReactSharedInternals.T = transition;
var currentTransition = ReactSharedInternals.T;

Expand All @@ -2985,11 +2981,13 @@ function startTransition(scope, options) {
{
try {
var returnValue = scope();
var onStartTransitionFinish = ReactSharedInternals.S;

if (onStartTransitionFinish !== null) {
onStartTransitionFinish(transition, returnValue);
}

if (typeof returnValue === 'object' && returnValue !== null && typeof returnValue.then === 'function') {
callbacks.forEach(function (callback) {
return callback(currentTransition, returnValue);
});
returnValue.then(noop, reportGlobalError);
}
} catch (error) {
Expand Down
19 changes: 9 additions & 10 deletions compiled/facebook-www/React-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ pureComponentPrototype.constructor = PureComponent;
assign(pureComponentPrototype, Component.prototype);
pureComponentPrototype.isPureReactComponent = !0;
var isArrayImpl = Array.isArray,
ReactSharedInternals = { H: null, A: null, T: null },
ReactSharedInternals = { H: null, A: null, T: null, S: null },
hasOwnProperty = Object.prototype.hasOwnProperty;
function getOwner() {
var dispatcher = ReactSharedInternals.A;
Expand Down Expand Up @@ -588,23 +588,22 @@ exports.memo = function (type, compare) {
};
exports.startTransition = function (scope, options) {
var prevTransition = ReactSharedInternals.T,
callbacks = new Set();
ReactSharedInternals.T = { _callbacks: callbacks };
var currentTransition = ReactSharedInternals.T;
transition = {};
ReactSharedInternals.T = transition;
enableTransitionTracing &&
void 0 !== options &&
void 0 !== options.name &&
((ReactSharedInternals.T.name = options.name),
(ReactSharedInternals.T.startTime = -1));
try {
var returnValue = scope();
var returnValue = scope(),
onStartTransitionFinish = ReactSharedInternals.S;
null !== onStartTransitionFinish &&
onStartTransitionFinish(transition, returnValue);
"object" === typeof returnValue &&
null !== returnValue &&
"function" === typeof returnValue.then &&
(callbacks.forEach(function (callback) {
return callback(currentTransition, returnValue);
}),
returnValue.then(noop, reportGlobalError));
returnValue.then(noop, reportGlobalError);
} catch (error) {
reportGlobalError(error);
} finally {
Expand Down Expand Up @@ -685,4 +684,4 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactSharedInternals.H.useTransition();
};
exports.version = "19.0.0-www-classic-e55b1679";
exports.version = "19.0.0-www-classic-abbf2e79";
19 changes: 9 additions & 10 deletions compiled/facebook-www/React-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ pureComponentPrototype.constructor = PureComponent;
assign(pureComponentPrototype, Component.prototype);
pureComponentPrototype.isPureReactComponent = !0;
var isArrayImpl = Array.isArray,
ReactSharedInternals = { H: null, A: null, T: null },
ReactSharedInternals = { H: null, A: null, T: null, S: null },
hasOwnProperty = Object.prototype.hasOwnProperty;
function getOwner() {
var dispatcher = ReactSharedInternals.A;
Expand Down Expand Up @@ -588,23 +588,22 @@ exports.memo = function (type, compare) {
};
exports.startTransition = function (scope, options) {
var prevTransition = ReactSharedInternals.T,
callbacks = new Set();
ReactSharedInternals.T = { _callbacks: callbacks };
var currentTransition = ReactSharedInternals.T;
transition = {};
ReactSharedInternals.T = transition;
enableTransitionTracing &&
void 0 !== options &&
void 0 !== options.name &&
((ReactSharedInternals.T.name = options.name),
(ReactSharedInternals.T.startTime = -1));
try {
var returnValue = scope();
var returnValue = scope(),
onStartTransitionFinish = ReactSharedInternals.S;
null !== onStartTransitionFinish &&
onStartTransitionFinish(transition, returnValue);
"object" === typeof returnValue &&
null !== returnValue &&
"function" === typeof returnValue.then &&
(callbacks.forEach(function (callback) {
return callback(currentTransition, returnValue);
}),
returnValue.then(noop, reportGlobalError));
returnValue.then(noop, reportGlobalError);
} catch (error) {
reportGlobalError(error);
} finally {
Expand Down Expand Up @@ -685,4 +684,4 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactSharedInternals.H.useTransition();
};
exports.version = "19.0.0-www-modern-e55b1679";
exports.version = "19.0.0-www-modern-abbf2e79";
19 changes: 9 additions & 10 deletions compiled/facebook-www/React-profiling.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pureComponentPrototype.constructor = PureComponent;
assign(pureComponentPrototype, Component.prototype);
pureComponentPrototype.isPureReactComponent = !0;
var isArrayImpl = Array.isArray,
ReactSharedInternals = { H: null, A: null, T: null },
ReactSharedInternals = { H: null, A: null, T: null, S: null },
hasOwnProperty = Object.prototype.hasOwnProperty;
function getOwner() {
var dispatcher = ReactSharedInternals.A;
Expand Down Expand Up @@ -592,23 +592,22 @@ exports.memo = function (type, compare) {
};
exports.startTransition = function (scope, options) {
var prevTransition = ReactSharedInternals.T,
callbacks = new Set();
ReactSharedInternals.T = { _callbacks: callbacks };
var currentTransition = ReactSharedInternals.T;
transition = {};
ReactSharedInternals.T = transition;
enableTransitionTracing &&
void 0 !== options &&
void 0 !== options.name &&
((ReactSharedInternals.T.name = options.name),
(ReactSharedInternals.T.startTime = -1));
try {
var returnValue = scope();
var returnValue = scope(),
onStartTransitionFinish = ReactSharedInternals.S;
null !== onStartTransitionFinish &&
onStartTransitionFinish(transition, returnValue);
"object" === typeof returnValue &&
null !== returnValue &&
"function" === typeof returnValue.then &&
(callbacks.forEach(function (callback) {
return callback(currentTransition, returnValue);
}),
returnValue.then(noop, reportGlobalError));
returnValue.then(noop, reportGlobalError);
} catch (error) {
reportGlobalError(error);
} finally {
Expand Down Expand Up @@ -689,7 +688,7 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactSharedInternals.H.useTransition();
};
exports.version = "19.0.0-www-classic-3337290d";
exports.version = "19.0.0-www-classic-d6ae41f2";
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
"function" ===
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&
Expand Down
19 changes: 9 additions & 10 deletions compiled/facebook-www/React-profiling.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pureComponentPrototype.constructor = PureComponent;
assign(pureComponentPrototype, Component.prototype);
pureComponentPrototype.isPureReactComponent = !0;
var isArrayImpl = Array.isArray,
ReactSharedInternals = { H: null, A: null, T: null },
ReactSharedInternals = { H: null, A: null, T: null, S: null },
hasOwnProperty = Object.prototype.hasOwnProperty;
function getOwner() {
var dispatcher = ReactSharedInternals.A;
Expand Down Expand Up @@ -592,23 +592,22 @@ exports.memo = function (type, compare) {
};
exports.startTransition = function (scope, options) {
var prevTransition = ReactSharedInternals.T,
callbacks = new Set();
ReactSharedInternals.T = { _callbacks: callbacks };
var currentTransition = ReactSharedInternals.T;
transition = {};
ReactSharedInternals.T = transition;
enableTransitionTracing &&
void 0 !== options &&
void 0 !== options.name &&
((ReactSharedInternals.T.name = options.name),
(ReactSharedInternals.T.startTime = -1));
try {
var returnValue = scope();
var returnValue = scope(),
onStartTransitionFinish = ReactSharedInternals.S;
null !== onStartTransitionFinish &&
onStartTransitionFinish(transition, returnValue);
"object" === typeof returnValue &&
null !== returnValue &&
"function" === typeof returnValue.then &&
(callbacks.forEach(function (callback) {
return callback(currentTransition, returnValue);
}),
returnValue.then(noop, reportGlobalError));
returnValue.then(noop, reportGlobalError);
} catch (error) {
reportGlobalError(error);
} finally {
Expand Down Expand Up @@ -689,7 +688,7 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactSharedInternals.H.useTransition();
};
exports.version = "19.0.0-www-modern-3337290d";
exports.version = "19.0.0-www-modern-d6ae41f2";
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
"function" ===
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&
Expand Down
Loading

0 comments on commit 4374cb6

Please sign in to comment.