fix(openAsync): resolve promise when closed externally

[p-iknow]

Summary

Fixes overlay.openAsync Promise not resolving when the overlay is closed externally (via overlay.close(id) rather than the internal close(value) callback).

Adds a defaultValue option to specify what value to resolve in such cases.

Closes #169

Screen.Recording.2026-01-23.at.6.01.42.PM.mov

---

🔴 PROBLEM

Core Issue

Item	Detail
Issue	overlay.openAsync() Promise never resolves when closed externally
Related Issue	#169 - Already reported

Requirements & Constraints

Requirement	Source	Description
No Breaking Changes	#169 Comment	Maintainer emphasized that any fix must not break existing behavior
Backward Compatible	Community feedback	Existing openAsync calls without options must continue to work unchanged
Opt-in Behavior	Design decision	New functionality should only activate when explicitly requested

Key Constraint from Issue #169:
The maintainer commented that the solution must be implemented without breaking changes.
This constraint led to the design choice of using an optional defaultValue parameter rather than changing the default behavior of openAsync.

Problem Scenario

const result = await overlay.openAsync<boolean>(({ isOpen, close }) => (
  <Dialog open={isOpen} onConfirm={() => close(true)} />
));
// ❌ This line NEVER executes when overlay.close(id) is called
console.log(result);

Close Method	Promise State
Internal close(value) call	✅ Resolved
External overlay.close(id) call	❌ Pending forever
External overlay.closeAll() call	❌ Pending forever
External overlay.unmount(id) call	❌ Pending forever

Real-World Scenarios Where This Occurs

1. Browser back button - Global handler calls overlay.close()
2. ESC key handler - External listener closes overlay programmatically
3. Auto-close timer - setTimeout(() => overlay.close(id), 5000)
4. Route change - Navigation cleanup closes all overlays
5. Parent component unmount - Cleanup effect calls overlay.close()

Technical Root Cause

// event.ts - The problematic code
const openAsync = async <T>(controller, options) => {
  return new Promise<T>((_resolve) => {
    open((overlayProps) => {
      // ⚠️ Only THIS close function resolves the Promise
      const close = (param: T) => {
        _resolve(param);      // ← Resolution happens ONLY here
        overlayProps.close();
      };
      return controller({ ...overlayProps, close });
    }, options);
  });
};

Call Flow Comparison:

[Internal close] close(true) → _resolve(true) → ✅ Promise resolved
[External close] overlay.close(id) → reducer CLOSE → isOpen=false → ❌ _resolve never called

Downstream Problems

Problem	Description
Memory Leak	Unresolved Promises and their closures remain in memory
Deadlock	await blocks indefinitely, subsequent code never runs
Poor UX	Application appears frozen after external close

---

🟢 ACTION

Design Rationale: Why defaultValue Option?

The solution uses an optional defaultValue parameter to satisfy the no breaking changes requirement from issue #169:

Approach Considered	Breaking Change?	Why Not Chosen
Always resolve with undefined on external close	✅ Yes	Changes return type, breaks existing code expecting specific values
Add new method openAsyncSafe()	❌ No	API fragmentation, confusing which to use
Add optional defaultValue parameter	❌ No	✅ Chosen - Opt-in, backward compatible

Why this works:

• Without defaultValue: Behavior unchanged (Promise stays pending on external close)
• With defaultValue: Promise resolves with specified value on external close
• Existing code requires zero modifications

Design Decisions

Item	Initial Plan	Final Implementation	Reason for Change
Option Name	onDismiss	defaultValue	Clearer semantic meaning
Events Monitored	close, closeAll	+ unmount, unmountAll	Cover all external close cases
Memory Management	Not considered	cleanup() function added	Prevent memory leaks
API Approach	New method (openAsyncSafe)	Add option to existing method	Maintain backward compatibility, per #169 requirement

---

⚠️ Technical Considerations: Event Subscription Outside React Hooks

Background: Hook vs External Subscription

Previously, overlay-kit only subscribed to events inside React hooks via useExternalEvents. This change introduces subscribeEvent to subscribe outside React's lifecycle (inside a Promise executor).

Aspect	Hook Subscription (useExternalEvents)	External Subscription (subscribeEvent)
Context	Inside React component	Outside React (Promise, callback, etc.)
Lifecycle	Tied to component mount/unmount	Manual management required
Cleanup	Automatic via useEffect return	Manual - must call unsubscribe
Memory Safety	React handles cleanup	Developer responsibility
Use Case	OverlayProvider listening to events	openAsync Promise waiting for close

Potential Risks Identified

1. Memory Leak Risk

Problem: Without React's automatic cleanup, event listeners could remain in memory indefinitely.

// ❌ RISK: If Promise never settles, subscription stays forever
const unsubscribe = subscribeEvent('close', handler);
// If we never call unsubscribe(), handler remains in emitter's Map

Mitigation:

// ✅ SOLUTION: cleanup() called on EVERY resolution path
const cleanup = () => {
  unsubscribeClose();
  unsubscribeCloseAll();
  unsubscribeUnmount();
  unsubscribeUnmountAll();
};

const resolve = (value: T) => {
  if (resolved) return;
  resolved = true;
  cleanup();  // Always cleanup before resolving
  _resolve(value);
};

const reject = (reason?: unknown) => {
  if (resolved) return;
  resolved = true;
  cleanup();  // Always cleanup before rejecting
  _reject(reason);
};

2. Double Resolution Risk

Problem: Both internal close(value) and external overlay.close(id) could try to resolve the same Promise.

// ❌ RISK: Race condition
// 1. User clicks confirm → close(true) → _resolve(true)
// 2. Simultaneously, external close event arrives → _resolve(defaultValue)
// Promise resolved twice! (Actually throws in strict mode)

Mitigation:

// ✅ SOLUTION: resolved flag ensures single resolution
let resolved = false;

const resolve = (value: T) => {
  if (resolved) return;  // Guard: ignore if already resolved
  resolved = true;
  cleanup();
  _resolve(value);
};

3. Handler Reference Management

Problem: emitter.off() requires the exact same function reference as emitter.on().

// ❌ RISK: Wrong reference = handler never removed
emitter.on('close', (id) => handler(id));
emitter.off('close', (id) => handler(id));  // Different function! Not removed.

Mitigation:

// ✅ SOLUTION: subscribeEvent captures reference in closure
function subscribeEvent(event, handler) {
  const eventKey = `${prefix}:${String(event)}`;
  const wrappedHandler = (payload) => handler(payload);  // Captured reference
  emitter.on(eventKey, wrappedHandler);
  return () => {
    emitter.off(eventKey, wrappedHandler);  // Same reference
  };
}

4. Subscription Timing (Race Condition)

Problem: Event might fire before subscription is established.

// ❌ RISK: Theoretical race condition
subscribeEvent('close', handler);  // Step 1: Subscribe
open(controller);                   // Step 2: Open overlay
// What if close event fires between Step 1 and Step 2?

Mitigation:

• JavaScript is single-threaded; synchronous code runs to completion
• Subscriptions are set up before open() is called
• No actual race condition possible in this synchronous flow

// ✅ SOLUTION: Subscription order guarantees safety
const unsubscribeClose = hasDefaultValue
  ? subscribeEvent('close', handler)  // 1. Subscribe first
  : () => {};

open(controller, { overlayId });       // 2. Then open
// Close event can only fire AFTER open() returns

5. Global Singleton Emitter

Problem: All overlay instances share one emitter. Could cause cross-contamination.

// ❌ RISK: Events from different overlays mixing
const emitter = createEmitter();  // Module-level singleton

Mitigation:

• Event keys are namespaced with prefix: ${prefix}:${event}
• Each overlay context has unique prefix
• Subscription handlers check overlayId before acting

// ✅ SOLUTION: ID-based filtering
subscribeEvent('close', (closedOverlayId: string) => {
  if (closedOverlayId === currentOverlayId) {  // Only react to OUR overlay
    resolve(options.defaultValue as T);
  }
});

6. Conditional Subscription (Unnecessary Listeners)

Problem: Subscribing when not needed wastes resources.

// ❌ RISK: Subscribing even when defaultValue not provided
const unsubscribe = subscribeEvent('close', handler);
// If no defaultValue, why subscribe at all?

Mitigation:

// ✅ SOLUTION: Only subscribe when defaultValue exists
const hasDefaultValue = options !== undefined && 'defaultValue' in options;

const unsubscribeClose = hasDefaultValue
  ? subscribeEvent('close', handler)
  : () => {};  // No-op if not needed

Summary: Risk Mitigation Matrix

Risk	Severity	Mitigation	Status
Memory Leak	🔴 High	cleanup() on all exit paths	✅ Resolved
Double Resolution	🔴 High	resolved flag guard	✅ Resolved
Handler Reference Mismatch	🟡 Medium	Closure captures reference	✅ Resolved
Race Condition (timing)	🟢 Low	Synchronous subscription before open	✅ N/A
Global Emitter Contamination	🟡 Medium	Namespace prefix + ID filtering	✅ Resolved
Unnecessary Subscriptions	🟢 Low	Conditional subscription	✅ Resolved

---

Core Implementation

1. New Option Type

type OpenAsyncOverlayOptions<T> = OpenOverlayOptions & {
  defaultValue?: T;  // 🆕 Value to resolve when closed externally
};

2. External Event Subscription (using subscribeEvent)

// Subscribe to events outside React hooks
const unsubscribeClose = hasDefaultValue
  ? subscribeEvent('close', (closedOverlayId) => {
      if (closedOverlayId === currentOverlayId) {
        resolve(options.defaultValue as T);
      }
    })
  : () => {};

3. Memory Leak Prevention

const cleanup = () => {
  unsubscribeClose();
  unsubscribeCloseAll();
  unsubscribeUnmount();
  unsubscribeUnmountAll();
};

const resolve = (value: T) => {
  if (resolved) return;  // Prevent double resolution
  resolved = true;
  cleanup();             // 🆕 Unsubscribe all listeners
  _resolve(value);
};

4. Conditional Subscription (Backward Compatibility)

const hasDefaultValue = options !== undefined && 'defaultValue' in options;

// No subscription without defaultValue → maintains existing behavior (pending)
const unsubscribeClose = hasDefaultValue ? subscribeEvent(...) : () => {};

Additional Issues Resolved

Pre-existing Issue	Solution
#169 - Promise pending forever	defaultValue option resolves on external close
Memory leak (unreported)	cleanup() function unsubscribes all listeners
Unmount events not handled (unreported)	Added unmount, unmountAll event subscriptions
Double resolution possible (unreported)	resolved flag prevents it

---

🔵 RESULT

Usage Example (Before vs After)

Before: The Problem

const result = await overlay.openAsync<boolean>(
  ({ isOpen, close }) => <Dialog onConfirm={() => close(true)} />
);
// When overlay.close(id) is called → ❌ Pending forever

After: The Solution

const result = await overlay.openAsync<boolean>(
  ({ isOpen, close }) => <Dialog onConfirm={() => close(true)} />,
  { defaultValue: false }  // 🆕 Resolves with false on external close
);

if (result === true) {
  console.log('User clicked confirm');
} else {
  console.log('Closed externally (ESC, back button, timeout, etc.)');
}

Test Coverage

Test Case	Status
Internal close(value) resolves with that value	✅
External overlay.close() resolves with defaultValue	✅
overlay.closeAll() resolves with defaultValue	✅
overlay.unmount() resolves with defaultValue	✅
overlay.unmountAll() resolves with defaultValue	✅
Without defaultValue, maintains existing behavior (pending)	✅
Prevents double resolution	✅
No memory leak due to subscription cleanup	✅

Key Benefits

Benefit	Description
Backward Compatible	Existing code works without any changes when defaultValue is not provided
Opt-in Behavior	Only add the option when needed
Complete Coverage	Handles close, closeAll, unmount, unmountAll
Memory Safe	Automatic subscription cleanup prevents leaks
Type Safe	Generic <T> applies to defaultValue as well

---

📐 Architecture Change (AS-IS / TO-BE)

Event Subscription Layer

AS-IS: Only React Hook subscription

flowchart TD
    subgraph LAYER["create-use-external-events.ts"]
        UEE["useExternalEvents()<br/>React Hook"]
        CE["createEvent()<br/>Event Dispatcher"]
    end
    
    subgraph EMITTER["Emitter"]
        ON["emitter.on()"]
        EMIT["emitter.emit()"]
    end
    
    UEE -->|"useLayoutEffect"| ON
    CE -->|"dispatch"| EMIT

Loading

TO-BE: Added external subscription function

flowchart TD
    subgraph LAYER["create-use-external-events.ts"]
        UEE["useExternalEvents()<br/>React Hook"]
        CE["createEvent()<br/>Event Dispatcher"]
        SE["subscribeEvent() 🆕<br/>External Subscription"]
    end
    
    subgraph EMITTER["Emitter"]
        ON["emitter.on()"]
        OFF["emitter.off()"]
        EMIT["emitter.emit()"]
    end
    
    UEE -->|"useLayoutEffect"| ON
    UEE -->|"cleanup"| OFF
    CE -->|"dispatch"| EMIT
    SE -->|"subscribe"| ON
    SE -->|"returns unsubscribe"| OFF

Loading

---

Promise Resolution Flow

AS-IS: External close → Promise pending forever

flowchart TD
    subgraph INTERNAL["Internal Close ✅"]
        I1["User clicks button"] --> I2["close(value)"]
        I2 --> I3["_resolve(value)"]
        I3 --> I4["Promise Resolved"]
    end
    
    subgraph EXTERNAL["External Close ❌"]
        E1["overlay.close(id)"] --> E2["emit 'close' event"]
        E2 --> E3["useOverlayEvent receives"]
        E3 --> E4["UI closes (isOpen=false)"]
        E4 --> E5["❌ _resolve never called"]
        E5 --> E6["Promise Pending Forever"]
    end
    
    style E5 fill:#f66,stroke:#333
    style E6 fill:#f66,stroke:#333

Loading

TO-BE: External close → Promise resolves with defaultValue

flowchart TD
    subgraph INTERNAL["Internal Close ✅"]
        I1["User clicks button"] --> I2["close(value)"]
        I2 --> I3["cleanup()"]
        I3 --> I4["_resolve(value)"]
        I4 --> I5["Promise Resolved"]
    end
    
    subgraph EXTERNAL["External Close ✅ 🆕"]
        E1["overlay.close(id)"] --> E2["emit 'close' event"]
        E2 --> E3["subscribeEvent handler triggered"]
        E3 --> E4{"resolved?"}
        E4 -->|"No"| E5["cleanup()"]
        E5 --> E6["_resolve(defaultValue)"]
        E6 --> E7["Promise Resolved with defaultValue"]
        E4 -->|"Yes"| E8["Skip (already resolved)"]
    end
    
    style E7 fill:#6f6,stroke:#333

Loading

---

Multiple Subscribers Pattern

AS-IS: Single subscriber (React Hook only)

flowchart LR
    E["emit('close', overlayId)"]
    E --> S1["OverlayProvider<br/>(useOverlayEvent)"]
    S1 --> R1["UI Update"]
    
    X["openAsync Promise"] -.->|"❌ No connection"| E

Loading

TO-BE: Multiple subscribers (Hook + External)

flowchart LR
    E["emit('close', overlayId)"]
    
    E --> S1["OverlayProvider<br/>(useOverlayEvent)"]
    E --> S2["openAsync 🆕<br/>(subscribeEvent)"]
    
    S1 --> R1["UI Update"]
    S2 --> R2["Promise Resolve"]
    
    style S2 fill:#9f9,stroke:#333
    style R2 fill:#9f9,stroke:#333

Loading

---

Data Flow Overview

AS-IS

flowchart TD
    USER["User Code<br/>overlay.openAsync()"]
    
    subgraph EVENT["Event Layer"]
        CREATE["createEvent()"]
        EMIT["emit()"]
    end
    
    subgraph REACT["React Layer"]
        HOOK["useOverlayEvent"]
        REDUCER["useReducer"]
    end
    
    UI["Overlay UI"]
    
    USER --> CREATE --> EMIT
    EMIT --> HOOK --> REDUCER --> UI
    
    USER -.->|"❌ No feedback<br/>on external close"| USER

Loading

TO-BE

flowchart TD
    USER["User Code<br/>overlay.openAsync()"]
    
    subgraph EVENT["Event Layer"]
        CREATE["createEvent()"]
        EMIT["emit()"]
        SUB["subscribeEvent() 🆕"]
    end
    
    subgraph REACT["React Layer"]
        HOOK["useOverlayEvent"]
        REDUCER["useReducer"]
    end
    
    UI["Overlay UI"]
    
    USER --> CREATE --> EMIT
    EMIT --> HOOK --> REDUCER --> UI
    EMIT --> SUB
    SUB -->|"✅ Resolve with<br/>defaultValue"| USER
    
    style SUB fill:#9f9,stroke:#333

Loading

---

Changes

• packages/src/event.ts - Add defaultValue option to openAsync
• packages/src/utils/create-use-external-events.ts - Add subscribeEvent function
• packages/src/event.test.tsx - Add comprehensive test cases
• examples/*/demo.tsx - Sync demo across React versions

Breaking Changes

None. This is a backward-compatible change.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 3a80a8e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel]

Someone is attempting to deploy a commit to the Toss Team on Vercel.

A member of the Team first needs to authorize it.

[Copilot]

Pull request overview

This PR fixes a critical issue where overlay.openAsync() promises never resolve when overlays are closed externally (via overlay.close(id), overlay.closeAll(), overlay.unmount(), or overlay.unmountAll()), which could lead to memory leaks and deadlocked code. The solution introduces an optional defaultValue parameter that enables the Promise to resolve with a specified value when external close events occur, while maintaining full backward compatibility.

Changes:

• Added subscribeEvent function to enable event subscription outside React's lifecycle
• Extended openAsync to accept a defaultValue option that resolves the Promise on external close/unmount
• Added comprehensive test coverage for all close/unmount scenarios
• Updated demo files across all React versions (16-19) to showcase the new functionality

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

File	Description
packages/src/utils/create-use-external-events.ts	Adds subscribeEvent function for manual event subscription outside React components with proper cleanup support
packages/src/event.ts	Implements defaultValue option for openAsync with event subscriptions for external close/unmount events and memory leak prevention
packages/src/event.test.tsx	Comprehensive test suite covering internal/external close, double resolution prevention, and backward compatibility
examples/react-{16,17,18,19}/framer-motion/src/demo.tsx	Identical demo implementations showcasing basic openAsync, defaultValue usage, and external close behavior

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[p-iknow]

soft remind @jungpaeng