Przeglądaj źródła

fix(host): review round 8 — unmount invalidation, unreadable selections clear

- HMR/unmount bumps both request generations, so a listing or creation
  settling after disposal neither updates dead state nor issues the
  post-create relist.
- A selection whose preview listing fails clears back to the single pane:
  the breadcrumb names the level, so the level must be the committing
  target (reverses the earlier keep-marked behavior with its test).
creatixchu 1 miesiąc temu
rodzic
commit
cbf54196fa

+ 9 - 0
packages/host/directory-picker-browse/src/client/DirectoryBrowser.tsx

@@ -124,6 +124,12 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
   // text input; the same guard the workspace-name inputs carry, shared by
   // the path editor and the folder-name input.
   const composingRef = useRef(false)
+  // HMR/unmount invalidation: a completion from a disposed flow must not
+  // update state or issue follow-up requests from a dead component.
+  useEffect(() => () => {
+    requestSeq.current += 1
+    openGeneration.current += 1
+  }, [])
   const compositionGuard = {
     onCompositionStart: () => { composingRef.current = true },
     onCompositionEnd: () => { composingRef.current = false },
@@ -163,6 +169,9 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
       if (seq !== requestSeq.current) return
       setLoading(false)
       setError(failureText(reason))
+      // An unreadable selection cannot be the committing target while the
+      // breadcrumb still names the level: fall back to the single pane.
+      setSelected(null)
     })
   }, [listDirectory])
 

+ 29 - 12
packages/host/directory-picker-browse/tests/directory-browser.spec.tsx

@@ -383,6 +383,35 @@ describe('DirectoryBrowser', () => {
     expect(screen.getByRole('button', { name: 'browser.editPath' })).toBeTruthy()
   })
 
+  it('drops a creation that settles after the browser unmounted', async () => {
+    let settleCreate!: (path: string) => void
+    const createDirectory = vi.fn(() => new Promise<string>((settle) => { settleCreate = settle }))
+    const b = mount({ createDirectory })
+    await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
+    fireEvent.click(screen.getByRole('button', { name: 'browser.newFolder' }))
+    fireEvent.change(screen.getByLabelText('browser.folderName'), { target: { value: 'slow' } })
+    fireEvent.click(screen.getByRole('button', { name: 'browser.create' }))
+    const listCalls = b.listDirectory.mock.calls.length
+    b.view.unmount()
+    // The dead flow must not issue the post-create relist.
+    await act(async () => { settleCreate(`${HOME}/slow`) })
+    expect(b.listDirectory.mock.calls.length).toBe(listCalls)
+  })
+
+  it('clears the selection when its preview listing fails', async () => {
+    const b = mount()
+    await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
+    b.listDirectory.mockImplementation(async () => {
+      throw new DirectoryBrowseError({ code: 'directory-unreadable', message: 'denied', details: { path: DOCS } })
+    })
+    fireEvent.click(rowButton(screen.getByRole('listitem')))
+    await waitFor(() => { expect(screen.getByRole('alert').textContent).toBe('denied') })
+    // The breadcrumb names the level, so the level must be the committing
+    // target: no half-selected two-pane state survives the failure.
+    expect(columns()).toHaveLength(1)
+    expect(rowButton(screen.getByRole('listitem')).getAttribute('aria-current')).toBeNull()
+  })
+
   it('ignores dismissal while adoption is busy', async () => {
     const b = mount({ busy: true })
     await waitFor(() => { expect(screen.getByRole('dialog')).toBeTruthy() })
@@ -508,18 +537,6 @@ describe('DirectoryBrowser', () => {
     expect(screen.getByRole('dialog', { name: 'browser.title' })).toBeTruthy()
   })
 
-  it('surfaces a selection-preview failure while keeping the selection marked', async () => {
-    const b = mount()
-    await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
-    b.listDirectory.mockRejectedValueOnce(
-      new DirectoryBrowseError({ code: 'directory-unreadable', message: 'denied', details: { path: DOCS } }))
-    fireEvent.click(rowButton(screen.getByRole('listitem')))
-    await waitFor(() => { expect(screen.getByRole('alert').textContent).toBe('denied') })
-    expect(rowButton(screen.getByRole('listitem')).getAttribute('aria-current')).toBe('true')
-    // No preview column arrived for the failed selection.
-    expect(columns()).toHaveLength(1)
-  })
-
   it('surfaces a post-create relist failure on the browser surface', async () => {
     const b = mount()
     await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })