fix and test loading ESM providers in VS Code (#54)

- Use a hacky pure-JS converter from ESM to CommonJS instead of the
esbuild approach, which was very heavyweight due to its use of wasm and
was hard for consumers of @openctx/vscode-lib to use.
- Refactor how clients can override imports.

.github/workflows/ci.yml

@@ -94,3 +94,6 @@ jobs:
         if: matrix.runner == 'ubuntu'
       - run: pnpm -C client/vscode run test:integration
         if: github.ref == 'refs/heads/main' && (matrix.runner == 'windows' || matrix.runner == 'macos')
+      - run: xvfb-run -a pnpm -C client/vscode run test:e2e
+        # if: matrix.runner == 'ubuntu' # only run e2e tests on Linux
+        if: false # no openctx logs printed so this fails for some reason on CI

bin/package.json

@@ -10,6 +10,6 @@
     "rxjs": "^7.8.1"
   },
   "devDependencies": {
-    "esbuild": "^0.19.11"
+    "esbuild": "^0.21.3"
   }
 }

client/browser/src/background/background.main.ts

@@ -37,7 +37,7 @@ function main(): void {
         configuration: () => configurationChanges,
         logger: console.error,
         makeRange: r => r,
-        dynamicImportFromUri: uri => Promise.resolve({ default: getBuiltinProvider(uri) }),
+        importProvider: uri => Promise.resolve({ default: getBuiltinProvider(uri) }),
     })
     subscriptions.add(() => client.dispose())
 

client/vscode-lib/src/controller.ts

@@ -2,7 +2,6 @@ import {
     type AuthInfo,
     type CapabilitiesParams,
     type Client,
-    type ClientEnv,
     type ItemsParams,
     type MentionsParams,
     type Range,
@@ -28,6 +27,7 @@ import { createHoverProvider } from './ui/editor/hover.js'
 import { createShowFileItemsList } from './ui/fileItemsList.js'
 import { createStatusBarItem } from './ui/statusBarItem.js'
 import { createErrorWaiter } from './util/errorWaiter.js'
+import { importProvider } from './util/importHelpers.js'
 import { observeWorkspaceConfigurationChanges, toEventEmitter } from './util/observable.js'
 
 export type VSCodeClient = Client<vscode.Range>
@@ -57,13 +57,11 @@ export function createController({
     secrets: secretsInput,
     outputChannel,
     getAuthInfo,
-    dynamicImportFromSource,
     features,
 }: {
     secrets: Observable<vscode.SecretStorage> | vscode.SecretStorage
     outputChannel: vscode.OutputChannel
     getAuthInfo?: (secrets: vscode.SecretStorage, providerUri: string) => Promise<AuthInfo | null>
-    dynamicImportFromSource?: ClientEnv<any>['dynamicImportFromSource']
     features: { annotations?: boolean; statusBar?: boolean }
 }): {
     controller: Controller
@@ -113,12 +111,7 @@ export function createController({
             : undefined,
         makeRange,
         logger: message => outputChannel.appendLine(message),
-
-        // On VS Code desktop, use a workaround for dynamic imports.
-        dynamicImportFromSource:
-            vscode.env.uiKind === vscode.UIKind.Desktop && process.env.DESKTOP_BUILD
-                ? dynamicImportFromSource
-                : undefined,
+        importProvider,
     })
 
     const errorTapObserver: Partial<TapObserver<any>> = {

client/vscode-lib/src/testing.ts

@@ -1,27 +1,11 @@
-import type { Annotation, Item, ItemsParams } from '@openctx/client'
-import type * as vscode from 'vscode'
 import type { Controller } from './controller.js'
 
 /**
  * The API exposed for this VS Code extension's tests.
  */
-export interface ExtensionApiForTesting {
-    /**
-     * Get OpenCtx items for the document.
-     */
-    getItems(params: ItemsParams): Promise<Item[] | null>
-
-    /**
-     * Get OpenCtx annotations for the document.
-     */
-    getAnnotations(
-        doc: Pick<vscode.TextDocument, 'uri' | 'getText'>
-    ): Promise<Annotation<vscode.Range>[] | null>
-}
+export interface ExtensionApiForTesting
+    extends Pick<Controller, 'capabilities' | 'items' | 'annotations'> {}
 
 export function createApiForTesting(controller: Controller): ExtensionApiForTesting {
-    return {
-        getItems: params => controller.items(params),
-        getAnnotations: doc => controller.annotations(doc),
-    }
+    return controller
 }

client/vscode-lib/src/util/esmToCommonJS.test.ts

@@ -0,0 +1,88 @@
+import { describe, expect, test } from 'vitest'
+import { esmToCommonJS } from './esmToCommonJS.js'
+
+describe('esmToCommonJS', () => {
+    test('default import statements', () => {
+        const esm = "import foo from './foo.js'"
+        const expected = "const foo = require('./foo.js');"
+        expect(esmToCommonJS(esm)).toBe(expected)
+    })
+
+    test('named import statements', () => {
+        const esm = "import { foo, bar } from './foobar.js'"
+        const expected = "const { foo, bar } = require('./foobar.js');"
+        expect(esmToCommonJS(esm)).toBe(expected)
+    })
+
+    test('namespace import statements', () => {
+        const esm = "import * as foobar from './foobar.js'"
+        const expected = "const foobar = require('./foobar.js');"
+        expect(esmToCommonJS(esm)).toBe(expected)
+    })
+
+    test('default export identifiers', () => {
+        const esm = 'export default foo'
+        const expected = 'module.exports = foo'
+        expect(esmToCommonJS(esm)).toBe(expected)
+    })
+
+    test('default export literals', () => {
+        const esm = 'export default { a: 123 }'
+        const expected = 'module.exports = { a: 123 }'
+        expect(esmToCommonJS(esm)).toBe(expected)
+    })
+
+    test('default export multi-line literals', () => {
+        const esm = 'export default { a: 123\nb: 456 }'
+        const expected = 'module.exports = { a: 123\nb: 456 }'
+        expect(esmToCommonJS(esm)).toBe(expected)
+    })
+
+    test('named export statements', () => {
+        const esm = 'export { foo, bar }'
+        const expected = 'module.exports = { foo, bar };'
+        expect(esmToCommonJS(esm)).toBe(expected)
+    })
+
+    test('named export statements with "as" keyword', () => {
+        const esm = 'export { foo as default }'
+        const expected = 'module.exports = foo;'
+        expect(esmToCommonJS(esm)).toBe(expected)
+    })
+
+    test('export const statements', () => {
+        const esm = 'export const foo = 123;'
+        const expected = 'exports.foo = 123;'
+        expect(esmToCommonJS(esm)).toBe(expected)
+    })
+
+    test('export function statements', () => {
+        const esm = 'export function foo(bar) {'
+        const expected = 'exports.foo = function foo(bar) {'
+        expect(esmToCommonJS(esm)).toBe(expected)
+    })
+
+    test('export class statements', () => {
+        const esm = 'export class Foo {'
+        const expected = 'exports.Foo = class Foo {'
+        expect(esmToCommonJS(esm)).toBe(expected)
+    })
+
+    test('minified code 1', () => {
+        const esm = 'var s=1;export{s as default}'
+        const expected = 'var s=1;module.exports = s;'
+        expect(esmToCommonJS(esm)).toBe(expected)
+    })
+
+    test('minified code 2', () => {
+        const esm = 'function(){}export{s as default}'
+        const expected = 'function(){}module.exports = s;'
+        expect(esmToCommonJS(esm)).toBe(expected)
+    })
+
+    test('minified code 3', () => {
+        const esm = 'export{x as y,s as default}'
+        const expected = 'module.exports = s;'
+        expect(esmToCommonJS(esm)).toBe(expected)
+    })
+})

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

@@ -0,0 +1,38 @@
+/**
+ * Convert from ESM JavaScript to CommonJS.
+ *
+ * NOTE(sqs): This is hacky! I hope VS Code starts supporting ESM import() in VS Code extensions
+ * soon so we can get rid of this! See https://github.com/microsoft/vscode/issues/130367.
+ *
+ * @param esm An ESM JavaScript string, assumed to have no import() calls.
+ */
+export function esmToCommonJS(esm: string): string {
+    // Convert import statements.
+    let cjs = esm.replace(/(?<=^|\b)import\s+(\w+)\s+from\s+['"](.+)['"]/gm, "const $1 = require('$2');")
+    cjs = cjs.replace(
+        /(?<=^|\b)import\s*\{\s*([\w\s,]+)\s*\}\s+from\s+['"](.+)['"]/gm,
+        "const { $1} = require('$2');"
+    )
+    cjs = cjs.replace(
+        /(?<=^|\b)import\s+\*\s+as\s+(\w+)\s+from\s+['"](.+)['"]/gm,
+        "const $1 = require('$2');"
+    )
+
+    // Convert export default statements.
+    cjs = cjs.replace(/(?<=^|\b)export\s+default\s+/gm, 'module.exports = ')
+
+    // Convert named export statements.
+    cjs = cjs.replace(
+        /(?<=^|\b)export\s*\{\s*(?:[^},]*,\s*)*([\w\s,]+) as default\s*\}/gm,
+        'module.exports = $1;'
+    )
+    cjs = cjs.replace(/(?<=^|\b)export\s*\{\s*([\w\s,]+)\s*\}/gm, 'module.exports = { $1};')
+    cjs = cjs.replace(/(?<=^|\b)export\s+const\s+(\w+)\s*=\s*(.+);/gm, 'exports.$1 = $2;')
+    cjs = cjs.replace(
+        /(?<=^|\b)export\s+function\s+(\w+)\s*\((.*)\)\s*\{/gm,
+        'exports.$1 = function $1($2) {'
+    )
+    cjs = cjs.replace(/(?<=^|\b)export\s+class\s+(\w+)\s*\{/gm, 'exports.$1 = class $1 {')
+
+    return cjs
+}

client/vscode-lib/src/util/importHelpers.test.ts

@@ -0,0 +1,14 @@
+import { describe, expect, test, vi } from 'vitest'
+import { importESMFromString } from './importHelpers.js'
+
+vi.mock('vscode', () => ({
+    env: { uiKind: 1 },
+    UIKind: { Desktop: 1 },
+}))
+
+describe('importESMFromString', () => {
+    test('works', async () =>
+        expect({ ...((await importESMFromString('export default 123')) as any) }).toEqual({
+            default: 123,
+        }))
+})

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

@@ -0,0 +1,67 @@
+import { readFile } from 'node:fs/promises'
+import { type Provider, fetchProviderSource } from '@openctx/client'
+import * as vscode from 'vscode'
+import { esmToCommonJS } from './esmToCommonJS.js'
+
+export async function importProvider(providerUri: string): Promise<{ default: Provider }> {
+    const url = new URL(providerUri)
+
+    let source: string
+    if (url.protocol === 'file:') {
+        source = await readFile(url.pathname, 'utf-8')
+    } else {
+        source = await fetchProviderSource(providerUri)
+    }
+
+    if (vscode.env.uiKind === vscode.UIKind.Desktop) {
+        // VS Code Desktop only supports require()ing of CommonJS modules.
+        return importCommonJSFromESM(source) as { default: Provider }
+    }
+
+    // VS Code Web supports import()ing, but not cross-origin.
+    return (await importESMFromString(source)) as { default: Provider }
+}
+
+/**
+ * Convert an ESM bundle to CommonJS.
+ *
+ * VS Code does not support dynamically import()ing ES modules (see
+ * https://github.com/microsoft/vscode/issues/130367). But we always want OpenCtx providers to be ES
+ * modules for consistency. So, we need to rewrite the ESM bundle to CommonJS to import it here.
+ *
+ * Note that it's deceiving because VS Code using extensionDevelopmentPath *does* support dynamic
+ * import()s, but they fail when installing the extension from a `.vsix` (including from the
+ * extension marketplace). When VS Code supports dynamic import()s for extensions, we can remove
+ * this.
+ */
+function importCommonJSFromESM(esmSource: string): unknown {
+    return {
+        default: requireCommonJSFromString('<CJS-STRING>', esmToCommonJS(esmSource)),
+    }
+}
+
+export function requireCommonJSFromString(filename: string, cjsSource: string): unknown {
+    const Module: any = module.constructor
+    const m = new Module()
+    m._compile(cjsSource, filename)
+    return m.exports
+}
+
+/**
+ * VS Code Web requires this because it blocks import()s of remote URLs, so we need to fetch() the
+ * source first and then import it by string.
+ */
+export function importESMFromString(esmSource: string): Promise<unknown> {
+    const url = `data:text/javascript;charset=utf-8;base64,${base64Encode(esmSource)}`
+    return import(/* @vite-ignore */ url)
+}
+
+/**
+ * See https://developer.mozilla.org/en-US/docs/Glossary/Base64#the_unicode_problem for why we need
+ * something other than just `btoa` for base64 encoding.
+ */
+function base64Encode(text: string): string {
+    const bytes = new TextEncoder().encode(text)
+    const binString = String.fromCodePoint(...bytes)
+    return btoa(binString)
+}

client/vscode-lib/tsconfig.json

@@ -3,6 +3,7 @@
   "compilerOptions": {
     "rootDir": "src",
     "outDir": "dist",
+    "lib": ["ESNext", "DOM"],
   },
   "include": ["src"],
   "exclude": ["dist"],

client/vscode/.vscodeignore

@@ -9,5 +9,4 @@ node_modules/
 tsconfig.json
 test/
 *.tsbuildinfo
-dev/**
-!node_modules/esbuild-wasm/esbuild.wasm
+dev/**

client/vscode/dev/build.mts

@@ -13,9 +13,10 @@ const buildTarget = getBuildTarget()
 const commonBuildOptions: esbuild.BuildOptions = {
     entryPoints: ['src/extension.ts'],
     bundle: true,
-    external: ['vscode', 'esbuild-wasm/esbuild.wasm', 'fs/promises'],
+    external: ['vscode'],
     format: 'cjs',
     sourcemap: true,
+    treeShaking: true,
     minify: false,
 }
 
@@ -25,13 +26,12 @@ const buildOptions: Record<BuildTarget, esbuild.BuildOptions> = {
         platform: 'node',
         outfile: 'dist/extension.node.cjs',
         outExtension: { '.js': '.cjs' },
-        define: { ...commonBuildOptions.define, 'process.env.DESKTOP_BUILD': 'true' },
     },
     web: {
         ...commonBuildOptions,
+        external: [...commonBuildOptions.external!, 'node:fs/promises'],
         platform: 'browser',
         outfile: 'dist/extension.web.js',
-        define: { ...commonBuildOptions.define, 'process.env.DESKTOP_BUILD': 'false' },
         alias: { ...commonBuildOptions.alias, path: 'path-browserify' },
     },
 }

client/vscode/dev/release.mts

@@ -95,33 +95,6 @@ execFileSync(
     }
 )
 
-// Add the esbuild wasm file.
-execFileSync('mkdir', ['-p', 'extension/node_modules/esbuild-wasm'], { stdio: 'inherit' })
-execFileSync(
-    'cp',
-    [
-        'node_modules/esbuild-wasm/esbuild.wasm',
-        'node_modules/esbuild-wasm/package.json',
-        'extension/node_modules/esbuild-wasm/',
-    ],
-    {
-        stdio: 'inherit',
-    }
-)
-execFileSync(
-    'zip',
-    [
-        '-ur',
-        'dist/openctx.vsix',
-        'extension/node_modules/esbuild-wasm/esbuild.wasm',
-        'extension/node_modules/esbuild-wasm/package.json',
-    ],
-    {
-        stdio: 'inherit',
-    }
-)
-execFileSync('rm', ['-rf', 'extension/node_modules/esbuild-wasm/'], { stdio: 'inherit' })
-
 // Publish the extension.
 console.error(`Publishing ${releaseType} release at version ${version}...`)
 if (dryRun) {