Răsfoiți Sursa

fix(tui): contain overlay reentrancy

ZiyaZhang 2 luni în urmă
părinte
comite
b64c3eb13f

+ 24 - 8
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<TuiOverlayOutcome> {
     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()

+ 77 - 8
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()