Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions lib/composables/dav.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,4 +240,53 @@ describe('dav composable', () => {
await waitRefLoaded(isLoading)
expect(abort).toBeCalledTimes(1)
})

it('a superseded (aborted) load does not clear the loading state of the newer load', async () => {
// Each getDirectoryContents call stays pending until resolved manually,
// and rejects with an AbortError when its signal is aborted — mirroring a
// real DAV request that is cancelled on fast navigation.
const pending: Array<{ resolve: () => void }> = []
const client = {
stat: vi.fn((v) => ({ data: { path: v } })),
getDirectoryContents: vi.fn((_path, opts) => new Promise((resolve, reject) => {
opts.signal?.addEventListener('abort', () => {
const error = new Error('aborted')
error.name = 'AbortError'
reject(error)
})
pending.push({ resolve: () => resolve({ data: [] }) })
})),
}
nextcloudFiles.getClient.mockImplementationOnce(() => client)
nextcloudFiles.resultToNode.mockImplementation((v) => v)

const view = ref<'files' | 'recent' | 'favorites'>('files')
const path = ref('/')
const { loadFiles, isLoading } = useDAVFiles(view, path)

// Load A is in flight
loadFiles()
await nextTick()
expect(pending).toHaveLength(1)
expect(isLoading.value).toBe(true)

// Load B supersedes A: navigating aborts A and starts B loading
path.value = '/subdir/'
await nextTick()
expect(pending).toHaveLength(2)

// Let A's aborted request reject and run its catch/finally
await nextTick()
await nextTick()

// Regression: A's finally must not flip isLoading to false while B is
// still loading — otherwise the FilePicker sees isLoading=false with a
// null folder and confirms an empty selection ("No nodes selected").
expect(isLoading.value).toBe(true)

// Completing B settles the loading state
pending[1].resolve()
await waitRefLoaded(isLoading)
expect(isLoading.value).toBe(false)
})
})
22 changes: 15 additions & 7 deletions lib/composables/dav.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,20 +68,20 @@ export function useDAVFiles(
async function loadDAVFiles() {
if (abortController) {
abortController.abort()
abortController = undefined
}

abortController = new AbortController()
const thisAbortController = new AbortController()
abortController = thisAbortController
isLoading.value = true
try {
if (currentView.value === 'favorites') {
files.value = await getFavoriteNodes({ client, path: currentPath.value, signal: abortController.signal })
files.value = await getFavoriteNodes({ client, path: currentPath.value, signal: thisAbortController.signal })
folder.value = null
} else if (currentView.value === 'recent') {
files.value = await getRecentNodes({ client, signal: abortController.signal })
files.value = await getRecentNodes({ client, signal: thisAbortController.signal })
folder.value = null
} else {
const content = await getNodes({ client, path: currentPath.value, signal: abortController.signal })
const content = await getNodes({ client, path: currentPath.value, signal: thisAbortController.signal })
folder.value = content.folder
files.value = content.contents
}
Expand All @@ -92,8 +92,16 @@ export function useDAVFiles(
}
throw error
} finally {
abortController = undefined
isLoading.value = false
// Only clear the shared loading state if this invocation is still the
// current load. When a newer loadDAVFiles() has already aborted and
// superseded this one, resetting here would flip `isLoading` to false
// (and drop the newer abort controller) while the newer load is still
// in flight and `folder` is still null — which lets the FilePicker
// confirm with an empty selection and throw "No nodes selected".
if (abortController === thisAbortController) {
abortController = undefined
isLoading.value = false
}
Comment on lines +101 to +104

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this can not just be simplified in general to only this change:

			if (abortController && abortController.signal.aborted) {
				abortController = undefined
				isLoading.value = false
			}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll check that.

}
}

Expand Down