remove AsyncGenerator API, improve Observable API (sourcegraph#195)

Async generators turned out to be too tricky to work with in the Cody
repo, so just remove that experimental async generator API. And make the
Observable API accept interop observables so we can use `observable-fns`
instead of full-blown `rxjs`.

client/vscode-lib/src/controller.test.ts

@@ -5,16 +5,12 @@ export function createMockController(): MockedObject<Controller> {
     return {
         observeMeta: vi.fn(),
         meta: vi.fn(),
-        metaChanges__asyncGenerator: vi.fn(),
         observeMentions: vi.fn(),
         mentions: vi.fn(),
-        mentionsChanges__asyncGenerator: vi.fn(),
         observeItems: vi.fn(),
         items: vi.fn(),
-        itemsChanges__asyncGenerator: vi.fn(),
         observeAnnotations: vi.fn(),
         annotations: vi.fn(),
-        annotationsChanges__asyncGenerator: vi.fn(),
     }
 }
 

client/vscode-lib/src/controller.ts

@@ -7,7 +7,18 @@ import {
     createClient,
 } from '@openctx/client'
 import type { ImportedProviderConfiguration } from '@openctx/client/src/configuration.js'
-import { type Observable, combineLatest, from, isObservable, map, mergeMap, of } from 'rxjs'
+import {
+    type InteropObservable,
+    type Observable,
+    type ObservableInput,
+    combineLatest,
+    from,
+    isObservable,
+    map,
+    mergeMap,
+    of,
+} from 'rxjs'
+import { isInteropObservable } from 'rxjs/internal/util/isInteropObservable'
 import * as vscode from 'vscode'
 import { getClientConfiguration } from './configuration.js'
 import { initializeOpenCtxGlobal } from './global.js'
@@ -26,15 +37,12 @@ type VSCodeClient = Client<vscode.Range>
 export interface Controller {
     observeMeta: VSCodeClient['metaChanges']
     meta: VSCodeClient['meta']
-    metaChanges__asyncGenerator: VSCodeClient['metaChanges__asyncGenerator']
 
     observeMentions: VSCodeClient['mentionsChanges']
     mentions: VSCodeClient['mentions']
-    mentionsChanges__asyncGenerator: VSCodeClient['mentionsChanges__asyncGenerator']
 
     observeItems: VSCodeClient['itemsChanges']
     items: VSCodeClient['items']
-    itemsChanges__asyncGenerator: VSCodeClient['itemsChanges__asyncGenerator']
 
     observeAnnotations(
         doc: Pick<vscode.TextDocument, 'uri' | 'getText'>,
@@ -43,11 +51,6 @@ export interface Controller {
         doc: Pick<vscode.TextDocument, 'uri' | 'getText'>,
         opts?: ProviderMethodOptions,
     ): ReturnType<VSCodeClient['annotations']>
-    annotationsChanges__asyncGenerator(
-        doc: Pick<vscode.TextDocument, 'uri' | 'getText'>,
-        opts?: ProviderMethodOptions,
-        signal?: AbortSignal,
-    ): ReturnType<VSCodeClient['annotationsChanges__asyncGenerator']>
 }
 
 export function createController({
@@ -60,15 +63,15 @@ export function createController({
     mergeConfiguration,
     preloadDelay,
 }: {
-    secrets: Observable<vscode.SecretStorage> | vscode.SecretStorage
+    secrets:
+        | Observable<vscode.SecretStorage>
+        | InteropObservable<vscode.SecretStorage>
+        | vscode.SecretStorage
     extensionId: string
     outputChannel: vscode.OutputChannel
     getAuthInfo?: (secrets: vscode.SecretStorage, providerUri: string) => Promise<AuthInfo | null>
     features: { annotations?: boolean; statusBar?: boolean }
-    providers?:
-        | ImportedProviderConfiguration[]
-        | Observable<ImportedProviderConfiguration[]>
-        | (() => AsyncGenerator<ImportedProviderConfiguration[]>)
+    providers?: ObservableInput<ImportedProviderConfiguration[]>
     mergeConfiguration?: (configuration: ClientConfiguration) => Promise<ClientConfiguration>
     preloadDelay?: number
 }): {
@@ -82,9 +85,10 @@ export function createController({
 
     const globalConfigurationChanges = observeWorkspaceConfigurationChanges('openctx')
 
-    const secrets: Observable<vscode.SecretStorage> = isObservable(secretsInput)
-        ? secretsInput
-        : of(secretsInput)
+    const secrets: Observable<vscode.SecretStorage> =
+        isObservable(secretsInput) || isInteropObservable(secretsInput)
+            ? from(secretsInput)
+            : of(secretsInput)
 
     // Watch for changes that could possibly affect configuration. This is overbroad because it does
     // not specify a config scope.
@@ -155,35 +159,19 @@ export function createController({
         UserAction.Implicit,
         client.annotationsChanges,
     )
-    const clientAnnotationsChanges__asyncGenerator = errorReporter.wrapAsyncGenerator(
-        UserAction.Implicit,
-        client.annotationsChanges__asyncGenerator,
-    )
 
     /**
      * The controller is passed to UI feature providers for them to fetch data.
      */
     const controller: Controller = {
         meta: errorReporter.wrapPromise(UserAction.Explicit, client.meta),
         observeMeta: errorReporter.wrapObservable(UserAction.Explicit, client.metaChanges),
-        metaChanges__asyncGenerator: errorReporter.wrapAsyncGenerator(
-            UserAction.Explicit,
-            client.metaChanges__asyncGenerator,
-        ),
 
         mentions: errorReporter.wrapPromise(UserAction.Explicit, client.mentions),
         observeMentions: errorReporter.wrapObservable(UserAction.Explicit, client.mentionsChanges),
-        mentionsChanges__asyncGenerator: errorReporter.wrapAsyncGenerator(
-            UserAction.Explicit,
-            client.mentionsChanges__asyncGenerator,
-        ),
 
         items: errorReporter.wrapPromise(UserAction.Explicit, client.items),
         observeItems: errorReporter.wrapObservable(UserAction.Explicit, client.itemsChanges),
-        itemsChanges__asyncGenerator: errorReporter.wrapAsyncGenerator(
-            UserAction.Explicit,
-            client.itemsChanges__asyncGenerator,
-        ),
 
         async annotations(doc: vscode.TextDocument, opts?: ProviderMethodOptions) {
             if (ignoreDoc(doc)) {
@@ -210,28 +198,6 @@ export function createController({
                 opts,
             )
         },
-        async *annotationsChanges__asyncGenerator(
-            doc: vscode.TextDocument,
-            opts?: ProviderMethodOptions,
-            signal?: AbortSignal,
-        ) {
-            if (ignoreDoc(doc)) {
-                return
-            }
-
-            const g = clientAnnotationsChanges__asyncGenerator(
-                {
-                    uri: doc.uri.toString(),
-                    content: doc.getText(),
-                },
-                opts,
-                signal,
-            )
-            for await (const v of g) {
-                yield v
-            }
-            return
-        },
     }
 
     if (features.annotations) {

client/vscode-lib/src/util/errorReporter.ts

@@ -67,46 +67,6 @@ export class ErrorReporterController implements vscode.Disposable {
         }
     }
 
-    /**
-     * wraps providerMethod to ensure it reports errors to the user.
-     */
-    public wrapAsyncGenerator<T, R>(
-        userAction: UserAction,
-        providerMethod: (
-            params: T,
-            opts?: ProviderMethodOptions,
-            signal?: AbortSignal,
-        ) => AsyncGenerator<R>,
-    ) {
-        const getErrorReporter = this.getErrorReporter.bind(this)
-        return async function* (
-            params: T,
-            opts?: ProviderMethodOptions,
-            signal?: AbortSignal,
-        ): AsyncGenerator<R> {
-            const errorReporter = getErrorReporter(userAction, opts)
-            if (errorReporter.skip) {
-                return
-            }
-
-            opts = withErrorHook(opts, (providerUri, error) => {
-                errorReporter.onError(providerUri, error)
-                errorReporter.report()
-            })
-
-            try {
-                for await (const value of providerMethod(params, opts, signal)) {
-                    errorReporter.onValue(undefined)
-                    errorReporter.report()
-                    yield value
-                }
-            } catch (error) {
-                errorReporter.onError(undefined, error)
-                errorReporter.report()
-            }
-        }
-    }
-
     /**
      * wraps providerMethod to ensure it reports errors to the user.
      */

lib/client/src/client/client.test.ts

@@ -1,6 +1,6 @@
 import type { AnnotationsParams, ItemsParams, MetaResult } from '@openctx/protocol'
 import type { Item, Range } from '@openctx/schema'
-import { Observable, firstValueFrom, of } from 'rxjs'
+import { firstValueFrom, of } from 'rxjs'
 import { TestScheduler } from 'rxjs/testing'
 import { describe, expect, test } from 'vitest'
 import type { Annotation, EachWithProviderUri } from '../api.js'
@@ -45,75 +45,114 @@ describe('Client', () => {
         new TestScheduler((actual, expected) => expect(actual).toStrictEqual(expected))
 
     describe('meta', () => {
-        test('meta with async generator providers option', async () => {
+        test('promise', async () => {
             const client = createTestClient({
-                configuration: () => of({}),
-                providers: async function* (): AsyncGenerator<ImportedProviderConfiguration[]> {
-                    yield [
-                        {
-                            provider: { meta: () => ({ name: 'my-provider-1' }) },
-                            providerUri: 'u1',
-                            settings: true,
+                configuration: () =>
+                    Promise.resolve({
+                        enable: true,
+                        providers: {
+                            [testdataFileUri('simple.js')]: true,
+                        },
+                    }),
+            })
+            expect(await client.meta({}, {})).toStrictEqual<EachWithProviderUri<MetaResult[]>>([
+                { name: 'simple', providerUri: testdataFileUri('simple.js'), annotations: {} },
+            ])
+        })
+
+        test('simple', () => {
+            testScheduler().run(({ cold, expectObservable }) => {
+                expectObservable(
+                    createTestClient({
+                        configuration: () =>
+                            cold<ConfigurationUserInput>('a', {
+                                a: {
+                                    enable: true,
+                                    providers: {
+                                        [testdataFileUri('simple.js')]: true,
+                                    },
+                                },
+                            }),
+                        __mock__: {
+                            getProviderClient: () => ({
+                                meta: (_, settings) => of({ name: 'simple' }),
+                            }),
+                        },
+                    }).metaChanges({}, {}),
+                ).toBe('a', {
+                    a: [{ name: 'simple', providerUri: testdataFileUri('simple.js') }],
+                } satisfies Record<string, EachWithProviderUri<MetaResult[]>>)
+            })
+        })
+
+        test('changes', () => {
+            testScheduler().run(({ cold, expectObservable }) => {
+                expectObservable(
+                    createTestClient({
+                        authInfo: () => cold('a', { a: null }),
+                        configuration: () =>
+                            cold<ConfigurationUserInput>('a-b-c', {
+                                a: { enable: false },
+                                b: {
+                                    enable: true,
+                                    providers: {
+                                        [testdataFileUri('simpleMeta.js')]: { nameSuffix: '1' },
+                                    },
+                                },
+                                c: {
+                                    enable: true,
+                                    providers: {
+                                        [testdataFileUri('simpleMeta.js')]: { nameSuffix: '2' },
+                                    },
+                                },
+                            }),
+                        providers: cold<ImportedProviderConfiguration[]>('a', { a: [] }),
+                        __mock__: {
+                            getProviderClient: () => ({
+                                meta: (_, settings) =>
+                                    of({ name: `simpleMeta-${settings.nameSuffix}`, annotations: {} }),
+                            }),
                         },
-                    ]
-                    await new Promise(resolve => setTimeout(resolve))
-                    yield [
+                    }).metaChanges({}, {}),
+                ).toBe('a-b-c', {
+                    a: [],
+                    b: [
                         {
-                            provider: { meta: () => ({ name: 'my-provider-2' }) },
-                            providerUri: 'u2',
-                            settings: true,
+                            name: 'simpleMeta-1',
+                            providerUri: testdataFileUri('simpleMeta.js'),
+                            annotations: {},
                         },
+                    ],
+                    c: [
                         {
-                            provider: { meta: () => ({ name: 'my-provider-3' }) },
-                            providerUri: 'u3',
-                            settings: true,
+                            name: 'simpleMeta-2',
+                            providerUri: testdataFileUri('simpleMeta.js'),
+                            annotations: {},
                         },
-                    ]
-                },
+                    ],
+                } satisfies Record<string, EachWithProviderUri<MetaResult[]>>)
             })
-
-            const values: EachWithProviderUri<MetaResult[]>[] = []
-            const signal = new AbortController().signal
-            for await (const value of client.metaChanges__asyncGenerator({}, {}, signal)) {
-                values.push(value)
-            }
-            expect(values).toStrictEqual<typeof values>([
-                [{ name: 'my-provider-1', providerUri: 'u1' }],
-                [{ name: 'my-provider-2', providerUri: 'u2' }],
-                [
-                    { name: 'my-provider-2', providerUri: 'u2' },
-                    { name: 'my-provider-3', providerUri: 'u3' },
-                ],
-            ])
         })
 
-        test('metaChanges__asyncGenerator', async () => {
+        test('providers option', async () => {
             const client = createTestClient({
-                configuration: () =>
-                    new Observable(observer => {
-                        observer.next({ enable: false, providers: {} })
-                        observer.next({
-                            enable: true,
-                            providers: { [testdataFileUri('simpleMeta.js')]: { nameSuffix: '1' } },
-                        })
-                        setTimeout(() => {
-                            observer.next({
-                                enable: true,
-                                providers: { [testdataFileUri('simpleMeta.js')]: { nameSuffix: '2' } },
-                            })
-                            observer.complete()
-                        })
-                    }),
+                configuration: () => of({ enable: true }),
+                providers: of([
+                    {
+                        provider: { meta: () => ({ name: 'my-provider-2' }) },
+                        providerUri: 'u2',
+                        settings: true,
+                    },
+                    {
+                        provider: { meta: () => ({ name: 'my-provider-3' }) },
+                        providerUri: 'u3',
+                        settings: true,
+                    },
+                ]),
             })
-
-            const values: EachWithProviderUri<MetaResult[]>[] = []
-            const signal = new AbortController().signal
-            for await (const value of client.metaChanges__asyncGenerator({}, {}, signal)) {
-                values.push(value)
-            }
-            expect(values).toStrictEqual<typeof values>([
-                [{ name: 'simpleMeta-1', providerUri: testdataFileUri('simpleMeta.js') }],
-                [{ name: 'simpleMeta-2', providerUri: testdataFileUri('simpleMeta.js') }],
+            expect(await client.meta({}, {})).toStrictEqual<EachWithProviderUri<MetaResult[]>>([
+                { name: 'my-provider-2', providerUri: 'u2' },
+                { name: 'my-provider-3', providerUri: 'u3' },
             ])
         })
     })