react
react copied to clipboard
[Fix] Simplify pendingState type in useActionState
Summary
This pull request simplifies the type definition of pendingState
in the useActionState
hook by changing it from Thenable<boolean> | boolean
to just boolean
.
The current implementation of useActionState
defines the type of pendingState
as Thenable<boolean> | boolean.
However, upon closer inspection of the code, it appears that pendingState
is always set to a boolean value, either directly or via the setPendingState
function, which only accepts a boolean argument.
pendingStateHook = mountStateImpl((false: Thenable<boolean> | boolean));
const setPendingState: boolean => void = (dispatchOptimisticSetState.bind(
null,
currentlyRenderingFiber,
false,
((pendingStateHook.queue: any): UpdateQueue<
S | Awaited<S>,
S | Awaited<S>,
>),
): any);
In the dispatchActionState
function, setPendingState
is always called with a boolean value:
setPendingState(true);
Therefore, defining pendingState
as Thenable<boolean> | boolean
seems unnecessary, as it's never actually set to a thenable value. This pull request proposes to simplify the type to boolean
, which more accurately reflects how pendingState
is used in practice.
This change improves code clarity and maintainability by removing the potentially confusing Thenable
type when it's not needed. It also aligns the type definition with the actual usage of pendingState
throughout the useActionState
implementation.
Comparison with useTransition
While the code for pendingState in useActionState appears to have been developed with reference to the useTransition hook, it's important to note that these are two different cases that require different handling.
In useTransition, isPending
can actually hold a thenable value, as evidenced by this code:
function updateTransition(): [
boolean,
(callback: () => void, options?: StartTransitionOptions) => void,
] {
const [booleanOrThenable] = updateState(false);
const hook = updateWorkInProgressHook();
const start = hook.memoizedState;
const isPending =
typeof booleanOrThenable === 'boolean'
? booleanOrThenable
: // This will suspend until the async action scope has finished.
useThenable(booleanOrThenable);
return [isPending, start];
}
Here, booleanOrThenable
is the result of updateState(false)
, and its type is Thenable<boolean> | boolean
. The value of isPending
depends on the type of booleanOrThenable
:
- If
booleanOrThenable
is a boolean,isPending
directly uses that value. - If
booleanOrThenable
is a thenable object,isPending
is assigned the result of processing the thenable withuseThenable
.
Therefore, in useTransition
, the Thenable<boolean> | boolean
type is appropriate for isPending
, as it can hold a thenable value.
In contrast, pendingState
in useActionState
is always used as a boolean value, so we can simplify its type to boolean
.
How did you test this change?
To verify that this change doesn't introduce any regressions, I ran the existing test suite with yarn test
. All tests passed without any issues, indicating that the updated type definition does not break any existing functionality.