diff --git a/packages/ui/tui/src/overlay-manager.ts b/packages/ui/tui/src/overlay-manager.ts index 643f344a57..efe8716777 100644 --- a/packages/ui/tui/src/overlay-manager.ts +++ b/packages/ui/tui/src/overlay-manager.ts @@ -52,6 +52,7 @@ interface OverlayEntry { readonly resolveClosed: (outcome: TuiOverlayOutcome) => void readonly session: TuiOverlaySession state: TuiOverlayState + component?: GuardedOverlayComponent handle?: OverlayHandle removeRequestAbort?: () => void outcome?: TuiOverlayOutcome @@ -130,11 +131,13 @@ class GuardedOverlayComponent implements Component, Focusable { } } - invalidate(): void { + invalidate(): boolean { try { this.component.invalidate() + return true } catch (error) { this.fail(error) + return false } } } @@ -242,11 +245,18 @@ export class TuiOverlayManager { this.fail(entry, error) return } + if (this.active !== entry) return const guarded = new GuardedOverlayComponent(component, (error) => { this.fail(entry, error) }) + entry.component = guarded try { - entry.handle = this.driver.show(guarded, entry.request.options) + const handle = this.driver.show(guarded, entry.request.options) + if (this.active !== entry) { + this.hide(handle) + return + } + entry.handle = handle this.driver.invalidate() } catch (error) { this.fail(entry, error) @@ -267,7 +277,8 @@ export class TuiOverlayManager { }, display: (value: string) => this.driver.display(value), invalidate: () => { - if (entry.state !== 'active') return + if (this.active !== entry || entry.component === undefined || entry.failing === true) return + if (!entry.component.invalidate() || this.active !== entry) return try { this.driver.invalidate() } catch (error) { @@ -295,6 +306,14 @@ export class TuiOverlayManager { } } + private hide(handle: OverlayHandle): void { + try { + handle.hide() + } catch (error) { + this.report(error) + } + } + private close(entry: OverlayEntry, result: TuiOverlayOutcome): Promise { if (entry.outcome !== undefined) return entry.closed entry.outcome = result @@ -306,13 +325,10 @@ export class TuiOverlayManager { if (queuedIndex >= 0) this.queue.splice(queuedIndex, 1) if (this.active === entry) { this.active = undefined - try { - entry.handle?.hide() - } catch (error) { - this.report(error) - } + if (entry.handle !== undefined) this.hide(entry.handle) delete entry.handle } + delete entry.component entry.resolveClosed(result) try { this.driver.invalidate() diff --git a/packages/ui/tui/tests/extension.spec.ts b/packages/ui/tui/tests/extension.spec.ts index 84248e3648..bf11d6c601 100644 --- a/packages/ui/tui/tests/extension.spec.ts +++ b/packages/ui/tui/tests/extension.spec.ts @@ -42,6 +42,7 @@ interface DriverFixture { errors: unknown[] invalidations: number showError?: unknown + onShow?: (component: Component) => void } function driverFixture(): DriverFixture { @@ -81,6 +82,7 @@ function driverFixture(): DriverFixture { }, isFocused: () => shown.focused, } + fixture.onShow?.(component) return handle }, invalidate() { @@ -154,11 +156,12 @@ describe('TuiOverlayManager', () => { expect(firstHost?.theme.accent('x')).toBe('accent:x') expect(firstHost?.display('\u001b')).toBe('safe:\u001b') firstHost?.invalidate() + expect(firstComponent.invalidated).toBe(1) expect(fixture.shown[0]?.component.render(40)).toEqual(['first:40']) fixture.shown[0]!.component.handleInput?.('x') fixture.shown[0]!.component.invalidate() expect(firstComponent.inputs).toEqual(['x']) - expect(firstComponent.invalidated).toBe(1) + expect(firstComponent.invalidated).toBe(2) expect(fixture.shown[0]?.component.wantsKeyRelease).toBe(true) ;(fixture.shown[0]?.component as Component & { focused: boolean }).focused = true expect(firstComponent.focused).toBe(true) @@ -244,6 +247,65 @@ describe('TuiOverlayManager', () => { expect(manager.hasActiveOverlay()).toBe(false) }) + it('does not mount entries closed or aborted during component construction', async () => { + const fixture = driverFixture() + const manager = new TuiOverlayManager(fixture.driver) + const closed = manager.open({ + create(host) { + host.invalidate() + host.close() + return component(['closed during construction']) + }, + }) + await expect(closed.closed).resolves.toEqual({ reason: 'closed' }) + + const controller = new AbortController() + const aborted = manager.open({ + signal: controller.signal, + create() { + controller.abort() + return component(['aborted during construction']) + }, + }) + await expect(aborted.closed).resolves.toEqual({ reason: 'aborted' }) + + const after = manager.open({ create: () => component(['after construction closes']) }) + expect(fixture.shown).toHaveLength(1) + expect(fixture.shown[0]?.component.render(40)).toEqual(['after construction closes']) + await after.close() + }) + + it('hides a handle returned after reentrant closure during mounting', async () => { + const fixture = driverFixture() + const manager = new TuiOverlayManager(fixture.driver) + fixture.onShow = (shown) => { + ;(shown as Component & { focused: boolean }).focused = true + } + const closed = manager.open({ + create(host) { + return { + get focused(): boolean { + return false + }, + set focused(_value: boolean) { + host.close() + }, + render: () => ['closed during mount'], + invalidate() {}, + } + }, + }) + await expect(closed.closed).resolves.toEqual({ reason: 'closed' }) + expect(fixture.shown[0]?.hidden).toBe(true) + expect(manager.hasActiveOverlay()).toBe(false) + + delete fixture.onShow + const after = manager.open({ create: () => component(['after mount close']) }) + expect(fixture.shown[1]?.hidden).toBe(false) + expect(fixture.shown[1]?.component.render(40)).toEqual(['after mount close']) + await after.close() + }) + it('stops admission and disposes active and queued overlays with the TUI', async () => { const fixture = driverFixture() const manager = new TuiOverlayManager(fixture.driver) @@ -315,15 +377,22 @@ describe('TuiOverlayManager', () => { await microtask() const invalidateError = new Error('invalidate failed') + let invalidatingHost: TuiOverlayHost | undefined const invalidating = manager.open({ - create: () => ({ - render: () => ['invalidate'], - invalidate() { - throw invalidateError - }, - }), + create(host) { + invalidatingHost = host + return { + render: () => ['invalidate'], + invalidate() { + throw invalidateError + }, + } + }, }) - fixture.shown.at(-1)!.component.invalidate() + const invalidationsBeforeFailure = fixture.invalidations + invalidatingHost?.invalidate() + invalidatingHost?.invalidate() + expect(fixture.invalidations).toBe(invalidationsBeforeFailure) expect(await invalidating.closed).toEqual({ reason: 'error', error: invalidateError }) await microtask()