Skip to content

Commit

Permalink
[crud] Narrow resource type
Browse files Browse the repository at this point in the history
Small refactor to the `resource` type to narrow it to an arbitrary object or void/null instead of the top type. This makes the overload on useEffect simpler since the return type of create is no longer widened to the top type when we merge their definitions.
  • Loading branch information
poteto committed Jan 24, 2025
1 parent de1eaa2 commit 1e059a3
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 52 deletions.
6 changes: 3 additions & 3 deletions packages/react-debug-tools/src/ReactDebugHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -739,11 +739,11 @@ function useHostTransitionStatus(): TransitionStatus {
}

function useResourceEffect(
create: () => mixed,
create: () => {...} | void | null,
createDeps: Array<mixed> | void | null,
update: ((resource: mixed) => void) | void,
update: ((resource: {...} | void | null) => void) | void,
updateDeps: Array<mixed> | void | null,
destroy: ((resource: mixed) => void) | void,
destroy: ((resource: {...} | void | null) => void) | void,
) {
nextHook();
hookLog.push({
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberCallUserSpace.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ export const callComponentWillUnmountInDEV: (
const callCreate = {
'react-stack-bottom-frame': function (
effect: Effect,
): (() => void) | mixed | void {
): (() => void) | {...} | void | null {
if (!enableUseResourceEffectHook) {
if (effect.resourceKind != null) {
if (__DEV__) {
Expand Down
8 changes: 5 additions & 3 deletions packages/react-reconciler/src/ReactFiberCommitEffects.js
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ export function commitHookEffectListMount(
addendum =
' You returned null. If your effect does not require clean ' +
'up, return undefined (or nothing).';
// $FlowFixMe (@poteto) this check is safe on arbitrary non-null/void objects
} else if (typeof destroy.then === 'function') {
addendum =
'\n\nIt looks like you wrote ' +
Expand Down Expand Up @@ -1036,10 +1037,10 @@ function safelyCallDestroy(
function safelyCallDestroyWithResource(
current: Fiber,
nearestMountedAncestor: Fiber | null,
destroy: mixed => void,
resource: mixed,
destroy: ({...}) => void,
resource: {...},
) {
const destroy_ = resource == null ? destroy : destroy.bind(null, resource);
const destroy_ = destroy.bind(null, resource);
if (__DEV__) {
runWithFiberInDEV(
current,
Expand All @@ -1050,6 +1051,7 @@ function safelyCallDestroyWithResource(
);
} else {
try {
// $FlowFixMe(incompatible-call) Already bound to resource
destroy_();
} catch (error) {
captureCommitPhaseError(current, nearestMountedAncestor, error);
Expand Down
78 changes: 39 additions & 39 deletions packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,8 @@ export type Hook = {
// the additional memory and we can follow up with performance
// optimizations later.
type EffectInstance = {
resource: mixed,
destroy: void | (() => void) | ((resource: mixed) => void),
resource: {...} | void | null,
destroy: void | (() => void) | ((resource: {...} | void | null) => void),
};

export const ResourceEffectIdentityKind: 0 = 0;
Expand All @@ -229,15 +229,15 @@ export type ResourceEffectIdentity = {
resourceKind: typeof ResourceEffectIdentityKind,
tag: HookFlags,
inst: EffectInstance,
create: () => mixed,
create: () => {...} | void | null,
deps: Array<mixed> | void | null,
next: Effect,
};
export type ResourceEffectUpdate = {
resourceKind: typeof ResourceEffectUpdateKind,
tag: HookFlags,
inst: EffectInstance,
update: ((resource: mixed) => void) | void,
update: ((resource: {...} | void | null) => void) | void,
deps: Array<mixed> | void | null,
next: Effect,
identity: ResourceEffectIdentity,
Expand Down Expand Up @@ -2540,9 +2540,9 @@ function pushResourceEffect(
identityTag: HookFlags,
updateTag: HookFlags,
inst: EffectInstance,
create: () => mixed,
create: () => {...} | void | null,
createDeps: Array<mixed> | void | null,
update: ((resource: mixed) => void) | void,
update: ((resource: {...} | void | null) => void) | void,
updateDeps: Array<mixed> | void | null,
): Effect {
const effectIdentity: ResourceEffectIdentity = {
Expand Down Expand Up @@ -2694,11 +2694,11 @@ function updateEffect(
}

function mountResourceEffect(
create: () => mixed,
create: () => {...} | void | null,
createDeps: Array<mixed> | void | null,
update: ((resource: mixed) => void) | void,
update: ((resource: {...} | void | null) => void) | void,
updateDeps: Array<mixed> | void | null,
destroy: ((resource: mixed) => void) | void,
destroy: ((resource: {...} | void | null) => void) | void,
) {
if (
__DEV__ &&
Expand Down Expand Up @@ -2730,11 +2730,11 @@ function mountResourceEffect(
function mountResourceEffectImpl(
fiberFlags: Flags,
hookFlags: HookFlags,
create: () => mixed,
create: () => {...} | void | null,
createDeps: Array<mixed> | void | null,
update: ((resource: mixed) => void) | void,
update: ((resource: {...} | void | null) => void) | void,
updateDeps: Array<mixed> | void | null,
destroy: ((resource: mixed) => void) | void,
destroy: ((resource: {...} | void | null) => void) | void,
) {
const hook = mountWorkInProgressHook();
currentlyRenderingFiber.flags |= fiberFlags;
Expand All @@ -2752,11 +2752,11 @@ function mountResourceEffectImpl(
}

function updateResourceEffect(
create: () => mixed,
create: () => {...} | void | null,
createDeps: Array<mixed> | void | null,
update: ((resource: mixed) => void) | void,
update: ((resource: {...} | void | null) => void) | void,
updateDeps: Array<mixed> | void | null,
destroy: ((resource: mixed) => void) | void,
destroy: ((resource: {...} | void | null) => void) | void,
) {
updateResourceEffectImpl(
PassiveEffect,
Expand All @@ -2772,11 +2772,11 @@ function updateResourceEffect(
function updateResourceEffectImpl(
fiberFlags: Flags,
hookFlags: HookFlags,
create: () => mixed,
create: () => {...} | void | null,
createDeps: Array<mixed> | void | null,
update: ((resource: mixed) => void) | void,
update: ((resource: {...} | void | null) => void) | void,
updateDeps: Array<mixed> | void | null,
destroy: ((resource: mixed) => void) | void,
destroy: ((resource: {...} | void | null) => void) | void,
) {
const hook = updateWorkInProgressHook();
const effect: Effect = hook.memoizedState;
Expand Down Expand Up @@ -4245,11 +4245,11 @@ if (__DEV__) {
if (enableUseResourceEffectHook) {
(HooksDispatcherOnMountInDEV: Dispatcher).useResourceEffect =
function useResourceEffect(
create: () => mixed,
create: () => {...} | void | null,
createDeps: Array<mixed> | void | null,
update: ((resource: mixed) => void) | void,
update: ((resource: {...} | void | null) => void) | void,
updateDeps: Array<mixed> | void | null,
destroy: ((resource: mixed) => void) | void,
destroy: ((resource: {...} | void | null) => void) | void,
): void {
currentHookNameInDev = 'useResourceEffect';
mountHookTypesDev();
Expand Down Expand Up @@ -4433,11 +4433,11 @@ if (__DEV__) {
if (enableUseResourceEffectHook) {
(HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).useResourceEffect =
function useResourceEffect(
create: () => mixed,
create: () => {...} | void | null,
createDeps: Array<mixed> | void | null,
update: ((resource: mixed) => void) | void,
update: ((resource: {...} | void | null) => void) | void,
updateDeps: Array<mixed> | void | null,
destroy: ((resource: mixed) => void) | void,
destroy: ((resource: {...} | void | null) => void) | void,
): void {
currentHookNameInDev = 'useResourceEffect';
updateHookTypesDev();
Expand Down Expand Up @@ -4620,11 +4620,11 @@ if (__DEV__) {
if (enableUseResourceEffectHook) {
(HooksDispatcherOnUpdateInDEV: Dispatcher).useResourceEffect =
function useResourceEffect(
create: () => mixed,
create: () => {...} | void | null,
createDeps: Array<mixed> | void | null,
update: ((resource: mixed) => void) | void,
update: ((resource: {...} | void | null) => void) | void,
updateDeps: Array<mixed> | void | null,
destroy: ((resource: mixed) => void) | void,
destroy: ((resource: {...} | void | null) => void) | void,
) {
currentHookNameInDev = 'useResourceEffect';
updateHookTypesDev();
Expand Down Expand Up @@ -4807,11 +4807,11 @@ if (__DEV__) {
if (enableUseResourceEffectHook) {
(HooksDispatcherOnRerenderInDEV: Dispatcher).useResourceEffect =
function useResourceEffect(
create: () => mixed,
create: () => {...} | void | null,
createDeps: Array<mixed> | void | null,
update: ((resource: mixed) => void) | void,
update: ((resource: {...} | void | null) => void) | void,
updateDeps: Array<mixed> | void | null,
destroy: ((resource: mixed) => void) | void,
destroy: ((resource: {...} | void | null) => void) | void,
) {
currentHookNameInDev = 'useResourceEffect';
updateHookTypesDev();
Expand Down Expand Up @@ -5019,11 +5019,11 @@ if (__DEV__) {
if (enableUseResourceEffectHook) {
(InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).useResourceEffect =
function useResourceEffect(
create: () => mixed,
create: () => {...} | void | null,
createDeps: Array<mixed> | void | null,
update: ((resource: mixed) => void) | void,
update: ((resource: {...} | void | null) => void) | void,
updateDeps: Array<mixed> | void | null,
destroy: ((resource: mixed) => void) | void,
destroy: ((resource: {...} | void | null) => void) | void,
): void {
currentHookNameInDev = 'useResourceEffect';
warnInvalidHookAccess();
Expand Down Expand Up @@ -5232,11 +5232,11 @@ if (__DEV__) {
if (enableUseResourceEffectHook) {
(InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).useResourceEffect =
function useResourceEffect(
create: () => mixed,
create: () => {...} | void | null,
createDeps: Array<mixed> | void | null,
update: ((resource: mixed) => void) | void,
update: ((resource: {...} | void | null) => void) | void,
updateDeps: Array<mixed> | void | null,
destroy: ((resource: mixed) => void) | void,
destroy: ((resource: {...} | void | null) => void) | void,
) {
currentHookNameInDev = 'useResourceEffect';
warnInvalidHookAccess();
Expand Down Expand Up @@ -5445,11 +5445,11 @@ if (__DEV__) {
if (enableUseResourceEffectHook) {
(InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).useResourceEffect =
function useResourceEffect(
create: () => mixed,
create: () => {...} | void | null,
createDeps: Array<mixed> | void | null,
update: ((resource: mixed) => void) | void,
update: ((resource: {...} | void | null) => void) | void,
updateDeps: Array<mixed> | void | null,
destroy: ((resource: mixed) => void) | void,
destroy: ((resource: {...} | void | null) => void) | void,
) {
currentHookNameInDev = 'useResourceEffect';
warnInvalidHookAccess();
Expand Down
6 changes: 3 additions & 3 deletions packages/react-reconciler/src/ReactInternalTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -398,11 +398,11 @@ export type Dispatcher = {
useEffectEvent?: <Args, F: (...Array<Args>) => mixed>(callback: F) => F,
// TODO: Non-nullable once `enableUseResourceEffectHook` is on everywhere.
useResourceEffect?: (
create: () => mixed,
create: () => {...} | void | null,
createDeps: Array<mixed> | void | null,
update: ((resource: mixed) => void) | void,
update: ((resource: {...} | void | null) => void) | void,
updateDeps: Array<mixed> | void | null,
destroy: ((resource: mixed) => void) | void,
destroy: ((resource: {...} | void | null) => void) | void,
) => void,
useInsertionEffect(
create: () => (() => void) | void,
Expand Down
6 changes: 3 additions & 3 deletions packages/react/src/ReactHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,11 @@ export function useEffectEvent<Args, F: (...Array<Args>) => mixed>(
}

export function useResourceEffect(
create: () => mixed,
create: () => {...} | void | null,
createDeps: Array<mixed> | void | null,
update: ((resource: mixed) => void) | void,
update: ((resource: {...} | void | null) => void) | void,
updateDeps: Array<mixed> | void | null,
destroy: ((resource: mixed) => void) | void,
destroy: ((resource: {...} | void | null) => void) | void,
): void {
if (!enableUseResourceEffectHook) {
throw new Error('Not implemented.');
Expand Down

0 comments on commit 1e059a3

Please sign in to comment.