feat(openAsync): add onDismiss option for external close handling

[p-iknow]

Summary

Adds an onDismiss option to overlay.openAsync that specifies a value to resolve when the overlay is closed externally (via overlay.close(id) or overlay.closeAll() rather than the internal close(value) callback).

Problem

When using openAsync, the Promise only resolves when the wrapped close(value) function is called from inside the overlay component.

However, if the overlay is closed externally via overlay.close(overlayId) or overlay.closeAll(), the Promise never resolves because the wrapped close function is never invoked.

Common external close scenarios:

• Browser back button navigation
• ESC key handlers
• Auto-close timers
• Route change cleanup
• Global state-driven close

Related issue: #169

Solution

Add optional onDismiss parameter that resolves when the overlay is closed externally:

const result = await overlay.openAsync<boolean | undefined>(
  ({ isOpen, close }) => (
    <Dialog
      open={isOpen}
      onConfirm={() => close(true)}
      onCancel={() => close(false)}
    />
  ),
  { onDismiss: undefined }
);

// result is:
// - true when user clicks confirm
// - false when user clicks cancel
// - undefined when closed externally (e.g., overlay.close(id))

Implementation Approach

Instead of trying to wrap or intercept the close behavior (which doesn't work because external closes bypass the wrapper), this implementation:

1. Subscribes to events: Uses subscribeEvent to listen for close and closeAll events
2. Detects external close: When the close event fires with matching overlayId, resolves with onDismiss value
3. Prevents double resolution: Guards against resolving twice if both internal and external close happen
4. Cleans up subscriptions: Unsubscribes when Promise is resolved or rejected

Changes

• Add subscribeEvent function to createUseExternalEvents for non-hook event subscription
• Add OpenAsyncOverlayOptions<T> type with optional onDismiss property
• Modify openAsync to subscribe to close/closeAll events and resolve with onDismiss when applicable
• Add comprehensive test cases covering all scenarios

Test Plan

New test cases added:

1. ✅ Resolves with close(value) when called internally (backward compatible)
2. ✅ Resolves with onDismiss when closed externally via overlay.close()
3. ✅ Resolves with onDismiss when closed via overlay.closeAll()
4. ✅ Stays pending without onDismiss option (backward compatible)
5. ✅ Prevents double resolution

Verification:

• yarn test - all 62 tests pass
• yarn build - builds successfully
• yarn lint - no lint errors
• yarn tsc --noEmit - no type errors

Breaking Changes

None. This is a backward-compatible change:

• Without onDismiss option: existing behavior (Promise stays pending on external close)
• With onDismiss option: Promise resolves with specified value on external close

Closes #169

[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 adds an onDismiss option to overlay.openAsync that allows the Promise to resolve with a specified value when the overlay is closed externally (via overlay.close(id) or overlay.closeAll()), rather than only when the internal close(value) callback is invoked. This addresses issue #169 where Promises from openAsync would never resolve when overlays were closed externally through browser back buttons, ESC key handlers, or route changes.

Changes:

• Added subscribeEvent function to enable non-hook event subscription for detecting external close events
• Introduced OpenAsyncOverlayOptions<T> type with optional onDismiss property for specifying the resolution value
• Refactored openAsync to subscribe to close/closeAll events and resolve with the onDismiss value when applicable, with double-resolution protection

Reviewed changes

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

File	Description
packages/src/utils/create-use-external-events.ts	Added subscribeEvent function for programmatic event subscription with proper cleanup
packages/src/event.ts	Added OpenAsyncOverlayOptions type and refactored openAsync to handle external close events via subscriptions
packages/src/event.test.tsx	Added comprehensive test suite covering internal close, external close via close/closeAll, backward compatibility, and double-resolution prevention

---

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

[Copilot]

Unsafe type casting: The expression options!.onDismiss as T could lead to type safety issues if onDismiss is explicitly set to a value that doesn't match the generic type T.

For example, if someone writes:

overlay.openAsync<boolean>(component, { onDismiss: "string" as any })

The type system wouldn't catch this at compile time if they bypass the type checker, and at runtime the Promise would resolve with a string even though the caller expects a boolean.

While TypeScript's type system should prevent this in most cases, the non-null assertion (!) combined with type casting (as T) bypasses some safety checks. Consider using a safer approach that validates the type or at minimum removing the non-null assertion since we've already checked that 'onDismiss' is in the options object.

Suggested change

	if (closedOverlayId === currentOverlayId && 'onDismiss' in (options ?? {})) {
	resolve(options!.onDismiss as T);
	}
	});
	const unsubscribeCloseAll = subscribeEvent('closeAll', () => {
	if ('onDismiss' in (options ?? {})) {
	resolve(options!.onDismiss as T);
	if (closedOverlayId === currentOverlayId && options && 'onDismiss' in options) {
	resolve(options.onDismiss as T);
	}
	});
	const unsubscribeCloseAll = subscribeEvent('closeAll', () => {
	if (options && 'onDismiss' in options) {
	resolve(options.onDismiss as T);

[Copilot]

Unsafe type casting: Same issue as line 55 - using options!.onDismiss as T could lead to type safety issues. The non-null assertion combined with type casting bypasses TypeScript's safety checks. Consider a safer approach or at minimum remove the non-null assertion since the 'onDismiss' key existence is already verified.

Suggested change

	resolve(options!.onDismiss as T);
	}
	});
	const unsubscribeCloseAll = subscribeEvent('closeAll', () => {
	if ('onDismiss' in (options ?? {})) {
	resolve(options!.onDismiss as T);
	resolve(options?.onDismiss as T);
	}
	});
	const unsubscribeCloseAll = subscribeEvent('closeAll', () => {
	if ('onDismiss' in (options ?? {})) {
	resolve(options?.onDismiss as T);

[Copilot]

Missing documentation: The new onDismiss option for openAsync is not documented in the README or in code comments. Users won't know about this feature unless they read the changelog or source code.

Consider adding:

1. A section in the README showing how to use onDismiss to handle external closes
2. JSDoc comments on the OpenAsyncOverlayOptions type explaining when and why to use onDismiss
3. An example showing a common use case like handling browser back button or ESC key dismissals

This is especially important since the behavior of openAsync changes subtly based on whether onDismiss is present (even if set to undefined).

[Copilot]

Memory leak: Event subscriptions are never cleaned up when the overlay is closed externally without onDismiss option.

When onDismiss is not provided and the overlay is closed externally (via overlay.close or overlay.closeAll), the Promise never resolves, but the event listeners created by subscribeEvent remain active indefinitely. This creates a memory leak that accumulates with each openAsync call without onDismiss.

The unsubscribeClose and unsubscribeCloseAll functions are only called when the Promise resolves or rejects, but if neither happens (the backward-compatible case), these subscriptions persist forever.

Consider adding cleanup logic that unsubscribes when the overlay is closed, regardless of whether onDismiss is provided. One approach would be to also listen for the close/closeAll events and clean up the subscriptions even when not resolving the Promise.