Fix issue with composeQuery (goplus#1247)

spx-gui/src/apis/common/index.ts

@@ -1,3 +1,4 @@
+import { mergeSignals } from '@/utils/disposable'
 import { Exception } from '@/utils/exception'
 import { Client } from './client'
 
@@ -28,6 +29,12 @@ class TimeoutException extends Exception {
   }
 }
 
+function getTimeoutSignal(timeout: number) {
+  const ctrl = new AbortController()
+  setTimeout(() => ctrl.abort(new TimeoutException()), timeout)
+  return ctrl.signal
+}
+
 export function useRequest(
   baseUrl: string = '',
   responseHandler?: ResponseHandler,
@@ -48,20 +55,10 @@ export function useRequest(
 
   return async function request(path: string, payload: unknown, options?: RequestOptions) {
     const req = await prepareRequest(path, payload, options)
+    options?.signal?.throwIfAborted()
     const timeout = options?.timeout ?? defaultTimeout
-    const ctrl = new AbortController()
-    if (options?.signal != null) {
-      // TODO: Reimplement this using `AbortSignal.any()` once it is widely supported.
-      options.signal.throwIfAborted()
-      options.signal.addEventListener('abort', () => ctrl.abort(options.signal?.reason))
-    }
-    const resp = await Promise.race([
-      fetch(req, { signal: ctrl.signal }),
-      new Promise<never>((_, reject) => setTimeout(() => reject(new TimeoutException()), timeout))
-    ]).catch((e) => {
-      if (e instanceof TimeoutException) ctrl.abort()
-      throw e
-    })
+    const signal = mergeSignals(options?.signal, getTimeoutSignal(timeout))
+    const resp = await fetch(req, { signal })
     return responseHandler != null ? responseHandler(resp) : resp
   }
 }

spx-gui/src/components/editor/code-editor/context.ts

@@ -151,15 +151,15 @@ export function useProvideCodeEditorCtx(
   })
 
   const editorQueryRet = useQuery<CodeEditor>(
-    async (signal) => {
+    async (ctx) => {
       const [project, runtime, monaco] = await Promise.all([
-        composeQuery(projectRet),
-        composeQuery(runtimeRet),
-        composeQuery(monacoQueryRet)
+        composeQuery(ctx, projectRet),
+        composeQuery(ctx, runtimeRet),
+        composeQuery(ctx, monacoQueryRet)
       ])
-      signal.throwIfAborted()
+      ctx.signal.throwIfAborted()
       const codeEditor = new CodeEditor(project, runtime, monaco, i18n)
-      codeEditor.disposeOnSignal(signal)
+      codeEditor.disposeOnSignal(ctx.signal)
       return codeEditor
     },
     { en: 'Failed to load code editor', zh: '加载代码编辑器失败' }

spx-gui/src/pages/community/project.vue

@@ -51,10 +51,10 @@ const {
   error,
   refetch: reloadProject
 } = useQuery(
-  async (signal) => {
+  async (ctx) => {
     const p = new Project()
     ;(window as any).project = p // for debug purpose, TODO: remove me
-    const loaded = await p.loadFromCloud(props.owner, props.name, true, signal)
+    const loaded = await p.loadFromCloud(props.owner, props.name, true, ctx.signal)
     return loaded
   },
   {

spx-gui/src/pages/editor/index.vue

@@ -83,11 +83,11 @@ const askToOpenTargetWithAnotherInCache = (targetName: string, cachedName: strin
 }
 
 const projectQueryRet = useQuery(
-  async (signal) => {
+  async (ctx) => {
     if (userInfo.value == null) throw new Error('User not signed in') // This should not happen as the route is protected
     // We need to read `userInfo.value?.name` & `projectName.value` synchronously,
     // so their change will drive `useQuery` to re-fetch
-    const project = await loadProject(userInfo.value.name, props.projectName, signal)
+    const project = await loadProject(userInfo.value.name, props.projectName, ctx.signal)
     ;(window as any).project = project // for debug purpose, TODO: remove me
     return project
   },
@@ -96,22 +96,22 @@ const projectQueryRet = useQuery(
 
 const project = projectQueryRet.data
 
-const runtimeQueryRet = useQuery(async (signal) => {
-  const project = await composeQuery(projectQueryRet)
-  signal.throwIfAborted()
+const runtimeQueryRet = useQuery(async (ctx) => {
+  const project = await composeQuery(ctx, projectQueryRet)
+  ctx.signal.throwIfAborted()
   const runtime = new Runtime(project)
-  runtime.disposeOnSignal(signal)
+  runtime.disposeOnSignal(ctx.signal)
   return runtime
 })
 
 const codeEditorQueryRet = useProvideCodeEditorCtx(projectQueryRet, runtimeQueryRet)
 
 const allQueryRet = useQuery(
-  (signal) =>
+  (ctx) =>
     Promise.all([
-      composeQuery(projectQueryRet, signal),
-      composeQuery(runtimeQueryRet, signal),
-      composeQuery(codeEditorQueryRet, signal)
+      composeQuery(ctx, projectQueryRet),
+      composeQuery(ctx, runtimeQueryRet),
+      composeQuery(ctx, codeEditorQueryRet)
     ]),
   { en: 'Failed to load editor', zh: '加载编辑器失败' }
 )

spx-gui/src/utils/disposable.ts

@@ -56,3 +56,19 @@ export function getCleanupSignal(onCleanup: OnCleanup) {
   onCleanup(() => ctrl.abort(new Cancelled('cleanup')))
   return ctrl.signal
 }
+
+// TODO: Reimplement this with `AbortSignal.any()` once it is widely supported (including Node.js version we're using) or properly polyfilled.
+export function mergeSignals(...signals: Array<AbortSignal | null | undefined>) {
+  const nonEmptySignals = signals.filter((s) => s != null) as AbortSignal[]
+  if (nonEmptySignals.length === 1) return nonEmptySignals[0]
+  const ctrl = new AbortController()
+  const resultSignal = ctrl.signal
+  for (const signal of nonEmptySignals) {
+    if (signal.aborted) {
+      ctrl.abort(signal.reason)
+      break
+    }
+    signal.addEventListener('abort', () => ctrl.abort(signal.reason), { signal: resultSignal })
+  }
+  return resultSignal
+}

spx-gui/src/utils/query.test.ts

@@ -0,0 +1,110 @@
+import { ref } from 'vue'
+import { flushPromises } from '@vue/test-utils'
+import { describe, it, expect, vi } from 'vitest'
+import { useQuery, composeQuery, type QueryContext } from './query'
+
+describe('composeQuery', () => {
+  it('should work well', async () => {
+    const valueRef = ref(0)
+    const queryFn1 = vi.fn(async () => valueRef.value)
+    const ret1 = useQuery(queryFn1)
+    const queryFn2 = vi.fn(async (ctx: QueryContext) => composeQuery(ctx, ret1))
+    const ret2 = useQuery(queryFn2)
+    expect(ret2.isLoading.value).toBe(true)
+    expect(ret2.data.value).toBe(null)
+    expect(ret2.error.value).toBe(null)
+
+    await flushPromises()
+    expect(ret2.isLoading.value).toBe(false)
+    expect(ret2.data.value).toBe(0)
+    expect(ret2.error.value).toBe(null)
+    expect(queryFn1).toHaveBeenCalledTimes(1)
+    expect(queryFn2).toHaveBeenCalledTimes(2)
+
+    valueRef.value++
+    await flushPromises()
+    expect(ret2.isLoading.value).toBe(false)
+    expect(ret2.data.value).toBe(1)
+    expect(ret2.error.value).toBe(null)
+    expect(queryFn1).toHaveBeenCalledTimes(2)
+    expect(queryFn2).toHaveBeenCalledTimes(4)
+
+    ret1.refetch()
+    await flushPromises()
+    expect(ret2.isLoading.value).toBe(false)
+    expect(ret2.data.value).toBe(1)
+    expect(ret2.error.value).toBe(null)
+    expect(queryFn1).toHaveBeenCalledTimes(3)
+    expect(queryFn2).toHaveBeenCalledTimes(6)
+
+    ret2.refetch()
+    await flushPromises()
+    expect(ret2.isLoading.value).toBe(false)
+    expect(ret2.data.value).toBe(1)
+    expect(ret2.error.value).toBe(null)
+    expect(queryFn1).toHaveBeenCalledTimes(4)
+    expect(queryFn2).toHaveBeenCalledTimes(9)
+  })
+
+  function makeErrorFn(times: number, err: unknown) {
+    return async () => {
+      times--
+      if (times >= 0) throw err
+      return 'ok'
+    }
+  }
+
+  it('should work well with exception', async () => {
+    const err = new Error('test')
+    const ret1 = useQuery(makeErrorFn(3, err))
+    const ret2 = useQuery(async (ctx: QueryContext) => composeQuery(ctx, ret1))
+
+    await flushPromises()
+    expect(ret2.isLoading.value).toBe(false)
+    expect(ret2.data.value).toBe(null)
+    expect(ret2.error.value).toBe(err)
+
+    ret2.refetch()
+    await flushPromises()
+    expect(ret2.isLoading.value).toBe(false)
+    expect(ret2.data.value).toBe(null)
+    expect(ret2.error.value).toBe(err)
+
+    ret1.refetch()
+    await flushPromises()
+    expect(ret2.isLoading.value).toBe(false)
+    expect(ret2.data.value).toBe(null)
+    expect(ret2.error.value).toBe(err)
+
+    ret2.refetch()
+    await flushPromises()
+    expect(ret2.isLoading.value).toBe(false)
+    expect(ret2.data.value).toBe('ok')
+    expect(ret2.error.value).toBe(null)
+  })
+
+  it('should work well with multiple dependencies', async () => {
+    const err1 = new Error('test1')
+    const err2 = new Error('test2')
+    const ret1 = useQuery(makeErrorFn(1, err1))
+    const ret2 = useQuery(makeErrorFn(2, err2))
+    const ret3 = useQuery(async (ctx: QueryContext) => Promise.all([composeQuery(ctx, ret1), composeQuery(ctx, ret2)]))
+
+    await flushPromises()
+    expect(ret3.isLoading.value).toBe(false)
+    expect(ret3.data.value).toBe(null)
+    expect(ret3.error.value).toBe(err1)
+
+    ret3.refetch()
+    await flushPromises()
+    expect(ret3.isLoading.value).toBe(false)
+    expect(ret3.data.value).toBe(null)
+    expect(ret3.error.value).toBe(err2)
+
+    ret3.refetch()
+    await flushPromises()
+    expect(ret3.isLoading.value).toBe(false)
+    expect(ret3.data.value).toEqual(['ok', 'ok'])
+    expect(ret3.error.value).toBe(null)
+  })
+})

spx-gui/src/utils/query.ts

@@ -1,8 +1,19 @@
-import { shallowRef, watchEffect, type Ref, type ShallowRef, type WatchSource, computed, ref, onUnmounted } from 'vue'
+import {
+  shallowRef,
+  watchEffect,
+  type Ref,
+  type ShallowRef,
+  type WatchSource,
+  computed,
+  ref,
+  onUnmounted,
+  toValue
+} from 'vue'
 import { useQuery as useVueQuery, useQueryClient as useVueQueryClient } from '@tanstack/vue-query'
 import { type LocaleMessage } from './i18n'
+import { mergeSignals } from './disposable'
 import { useAction, type ActionException, Cancelled } from './exception'
-import { timeout, until } from './utils'
+import { until } from './utils'
 
 export type QueryRet<T> = {
   isLoading: Ref<boolean>
@@ -11,14 +22,28 @@ export type QueryRet<T> = {
   refetch: (signal?: AbortSignal) => void
 }
 
+/**
+ * Source of query triggering.
+ * - `auto`: query is triggered automatically for dependencies' change.
+ * - `refetch`: query is triggered manually by `refetch`.
+ */
+export type QuerySource = 'auto' | 'refetch'
+
+export type QueryContext = {
+  /** Signal for aborting the query. */
+  signal: AbortSignal
+  /** Source of query fn calling. */
+  source: QuerySource
+}
+
 /**
  * `useQuery`
  * - do query automatically
  * - transform exceptions like `useAction`
  * - manage states for query result
  */
 export function useQuery<T>(
-  queryFn: (signal: AbortSignal) => Promise<T>,
+  queryFn: (ctx: QueryContext) => Promise<T>,
   failureSummaryMessage?: LocaleMessage
 ): QueryRet<T> {
   if (failureSummaryMessage != null) {
@@ -37,10 +62,10 @@ export function useQuery<T>(
     return ctrl.signal
   }
 
-  function fetch() {
-    const signal = getSignal()
+  function fetch(source: QuerySource, signal?: AbortSignal) {
+    const finalSignal = mergeSignals(signal, getSignal())
     isLoading.value = true
-    queryFn(signal).then(
+    queryFn({ signal: finalSignal, source }).then(
       (d) => {
         data.value = d
         error.value = null
@@ -55,9 +80,11 @@ export function useQuery<T>(
     )
   }
 
-  watchEffect(fetch)
+  watchEffect(() => fetch('auto'))
+
+  const refetch = (signal?: AbortSignal) => fetch('refetch', signal)
 
-  return { isLoading, data, error, refetch: fetch }
+  return { isLoading, data, error, refetch }
 }
 
 export type QueryWithCacheOptions<T> = {
@@ -107,24 +134,20 @@ export function useQueryCache<T>() {
 }
 
 /**
- * Compose query.
+ * Compose query in another query.
  * - If the query is loading, wait until it's done.
  * - If the query failed, error will be thrown.
  * - If the query is successful, the data will be returned.
  * - Composed query will be collected as dependencies.
  */
-export async function composeQuery<T>(queryRet: QueryRet<T>, signal?: AbortSignal): Promise<T> {
-  // Trigger failed query to refetch. `timeout(0)` to avoid dependency cycle.
-  timeout(0).then(() => {
-    if (!queryRet.isLoading.value && queryRet.error.value != null) {
-      queryRet.refetch(signal)
-    }
-  })
-
-  queryRet.isLoading.value // Trigger dependency collection
-
+export async function composeQuery<T>(ctx: QueryContext, queryRet: QueryRet<T>): Promise<T> {
+  if (ctx.source === 'refetch') {
+    queryRet.refetch(ctx.signal)
+  } else {
+    toValue(queryRet.isLoading) // Trigger dependency collection
+  }
   return new Promise<T>((resolve, reject) => {
-    until(() => !queryRet.isLoading.value, signal).then(() => {
+    until(() => !queryRet.isLoading.value, ctx.signal).then(() => {
       if (queryRet.error.value != null) reject(queryRet.error.value)
       else resolve(queryRet.data.value!)
     })