Skip to content

fix(useAsyncEffect): call cleanup when component unmounts before async effect resolves #351

Open
eunwoo-levi wants to merge 3 commits intotoss:mainfrom
eunwoo-levi:fix/use-async-effect-cleanup-race-condition
Open

fix(useAsyncEffect): call cleanup when component unmounts before async effect resolves #351
eunwoo-levi wants to merge 3 commits intotoss:mainfrom
eunwoo-levi:fix/use-async-effect-cleanup-race-condition

Conversation

@eunwoo-levi
Copy link
Contributor

Overview

useAsyncEffect drops the cleanup function when the component unmounts before the Promise resolves.

Root Cause

cleanup is assigned inside .then(), but React's cleanup runs synchronously on unmount.
If unmount fires before the Promise resolves, cleanup is still undefined — the user's cleanup function is permanently lost.

effect().then(result => {
  cleanup = result; // assigned asynchronously
});                                                                                       

return () => {                                                                            
  cleanup?.(); // undefined if unmount happens first → no-op
};

Fix

Added an isCleaned flag. If unmount fires before the Promise resolves, the cleanup is called immediately after the Promise settles.

effect().then(result => {
  cleanup = result;
  if (isCleaned) {                                                                        
    cleanup?.();
  }                                                                                       
});             

return () => {
  isCleaned = true;
  cleanup?.();
};

Changes

  • useAsyncEffect.ts — add isCleaned flag
  • useAsyncEffect.spec.ts — add missing test case: unmount before Promise resolves

Checklist

  • Did you write the test code?
  • Have you run yarn run fix to format and lint the code and docs?
  • Have you run yarn run test:coverage to make sure there is no uncovered line?
  • Did you write the JSDoc?

eunwoo-levi and others added 2 commits March 18, 2026 14:32
… resolves

The cleanup function returned from the async effect was silently lost
if the component unmounted before the Promise resolved. Added an
`isCleaned` flag so the cleanup is invoked immediately after the
Promise settles in that case.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers the case where the component unmounts before the async effect's
Promise resolves — the previously missing scenario that allowed the
race condition bug to go undetected.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 18, 2026 05:53
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes useAsyncEffect so that a cleanup function returned from an async effect is still executed even if the component unmounts (or deps change) before the promise resolves.

Changes:

  • Track an isCleaned flag to ensure late-resolving cleanup functions are invoked after unmount.
  • Add a regression test covering “unmount before promise resolves”.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
packages/core/src/hooks/useAsyncEffect/useAsyncEffect.ts Ensures async-provided cleanup still runs if unmount happens before the promise settles.
packages/core/src/hooks/useAsyncEffect/useAsyncEffect.spec.ts Adds a regression test for early unmount with delayed async resolution.

@@ -24,12 +24,17 @@ import { DependencyList, useEffect } from 'react';
export function useAsyncEffect(effect: () => Promise<void | (() => void)>, deps?: DependencyList) {
useEffect(() => {
Comment on lines 29 to 34
effect().then(result => {
cleanup = result;
if (isCleaned) {
cleanup?.();
}
});
Comment on lines 24 to 25
export function useAsyncEffect(effect: () => Promise<void | (() => void)>, deps?: DependencyList) {
useEffect(() => {
@eunwoo-levi eunwoo-levi changed the title fix(core): call cleanup when component unmounts before async effect resolves fix(useAsyncEffect): call cleanup when component unmounts before async effect resolves Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants