From 11bfd4eadf6f4b79ee6a06c1f6fc127322513c69 Mon Sep 17 00:00:00 2001 From: Darshan-Naik Date: Sun, 23 Nov 2025 13:42:46 +0530 Subject: [PATCH] Add runtime type check before calling useEffect cleanup functions This prevents runtime errors in production when non-function cleanup values are stored. The check ensures cleanup functions are only called when they are actually functions, preventing TypeError crashes. - Add typeof check in commitHookEffectListUnmount before calling cleanup - Add defensive check in safelyCallDestroy as a safety net - Prevents unnecessary profiling overhead for non-function values --- .../src/ReactFiberCommitEffects.js | 45 ++++++++++--------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js index f7e20fe926193..d12b1545cf422 100644 --- a/packages/react-reconciler/src/ReactFiberCommitEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -264,31 +264,33 @@ export function commitHookEffectListUnmount( const destroy = inst.destroy; if (destroy !== undefined) { inst.destroy = undefined; - if (enableSchedulingProfiler) { - if ((flags & HookPassive) !== NoHookEffect) { - markComponentPassiveEffectUnmountStarted(finishedWork); - } else if ((flags & HookLayout) !== NoHookEffect) { - markComponentLayoutEffectUnmountStarted(finishedWork); + if (typeof destroy === 'function') { + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectUnmountStarted(finishedWork); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectUnmountStarted(finishedWork); + } } - } - if (__DEV__) { - if ((flags & HookInsertion) !== NoHookEffect) { - setIsRunningInsertionEffect(true); + if (__DEV__) { + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(true); + } } - } - safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); - if (__DEV__) { - if ((flags & HookInsertion) !== NoHookEffect) { - setIsRunningInsertionEffect(false); + safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); + if (__DEV__) { + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(false); + } } - } - if (enableSchedulingProfiler) { - if ((flags & HookPassive) !== NoHookEffect) { - markComponentPassiveEffectUnmountStopped(); - } else if ((flags & HookLayout) !== NoHookEffect) { - markComponentLayoutEffectUnmountStopped(); + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectUnmountStopped(); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectUnmountStopped(); + } } } } @@ -912,6 +914,9 @@ function safelyCallDestroy( destroy: (() => void) | (({...}) => void), resource?: {...} | void | null, ) { + if (typeof destroy !== 'function') { + return; + } // $FlowFixMe[extra-arg] @poteto this is safe either way because the extra arg is ignored if it's not a CRUD effect const destroy_ = resource == null ? destroy : destroy.bind(null, resource); if (__DEV__) {