Skip to content

Commit 2bb1697

Browse files
darthmolenSteveSandersonMS
authored andcommitted
fix(nodejs): add CJS compatibility for VS Code extensions (#528)
Replace import.meta.resolve with createRequire + path walking in getBundledCliPath(). The new implementation falls back to __filename when import.meta.url is unavailable (shimmed CJS environments like VS Code extensions bundled with esbuild format:"cjs"). Single ESM build output retained — no dual CJS/ESM builds needed. The fallback logic handles both native ESM and shimmed CJS contexts.
1 parent 0fbe0f6 commit 2bb1697

2 files changed

Lines changed: 81 additions & 7 deletions

File tree

nodejs/src/client.ts

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@
1414
import { spawn, type ChildProcess } from "node:child_process";
1515
import { randomUUID } from "node:crypto";
1616
import { existsSync } from "node:fs";
17+
import { createRequire } from "node:module";
1718
import { Socket } from "node:net";
1819
import { dirname, join } from "node:path";
19-
import { fileURLToPath } from "node:url";
20+
import { pathToFileURL } from "node:url";
2021
import {
2122
createMessageConnection,
2223
MessageConnection,
@@ -91,14 +92,29 @@ function getNodeExecPath(): string {
9192
/**
9293
* Gets the path to the bundled CLI from the @github/copilot package.
9394
* Uses index.js directly rather than npm-loader.js (which spawns the native binary).
95+
*
96+
* The @github/copilot package only exposes an ESM-only "./sdk" export,
97+
* which breaks in CJS contexts (e.g., VS Code extensions bundled with esbuild).
98+
* Instead of resolving through the package's exports, we locate the package
99+
* root by walking module resolution paths and checking for its directory.
100+
* See: https://github.com/github/copilot-sdk/issues/528
94101
*/
95102
function getBundledCliPath(): string {
96-
// Find the actual location of the @github/copilot package by resolving its sdk export
97-
const sdkUrl = import.meta.resolve("@github/copilot/sdk");
98-
const sdkPath = fileURLToPath(sdkUrl);
99-
// sdkPath is like .../node_modules/@github/copilot/sdk/index.js
100-
// Go up two levels to get the package root, then append index.js
101-
return join(dirname(dirname(sdkPath)), "index.js");
103+
// import.meta.url is defined in ESM; in CJS bundles (esbuild format:"cjs")
104+
// it's undefined, so we fall back to __filename via pathToFileURL.
105+
const require = createRequire(import.meta.url ?? pathToFileURL(__filename).href);
106+
// The @github/copilot package has strict ESM-only exports, so require.resolve
107+
// cannot resolve it. Instead, walk the module resolution paths to find it.
108+
const searchPaths = require.resolve.paths("@github/copilot") ?? [];
109+
for (const base of searchPaths) {
110+
const candidate = join(base, "@github", "copilot", "index.js");
111+
if (existsSync(candidate)) {
112+
return candidate;
113+
}
114+
}
115+
throw new Error(
116+
`Could not find @github/copilot package. Searched ${searchPaths.length} paths. Ensure it is installed.`
117+
);
102118
}
103119

104120
/**

nodejs/test/cjs-compat.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/**
2+
* CJS shimmed environment compatibility test
3+
*
4+
* Verifies that getBundledCliPath() works when the ESM build is loaded in a
5+
* shimmed CJS environment (e.g., VS Code extensions bundled with esbuild
6+
* format:"cjs"). In these environments, import.meta.url may be undefined but
7+
* __filename is available via the CJS shim.
8+
*
9+
* See: https://github.com/github/copilot-sdk/issues/528
10+
*/
11+
12+
import { describe, expect, it } from "vitest";
13+
import { existsSync } from "node:fs";
14+
import { execFileSync } from "node:child_process";
15+
import { join } from "node:path";
16+
17+
const esmEntryPoint = join(import.meta.dirname, "../dist/index.js");
18+
19+
describe("CJS shimmed environment compatibility (#528)", () => {
20+
it("ESM dist file should exist", () => {
21+
expect(existsSync(esmEntryPoint)).toBe(true);
22+
});
23+
24+
it("getBundledCliPath() should resolve in a CJS shimmed context", () => {
25+
// Simulate what esbuild format:"cjs" does: __filename is defined,
26+
// import.meta.url may be undefined. The SDK's fallback logic
27+
// (import.meta.url ?? pathToFileURL(__filename).href) handles this.
28+
//
29+
// We test by requiring the ESM build via --input-type=module in a
30+
// subprocess that has __filename available, verifying the constructor
31+
// (which calls getBundledCliPath()) doesn't throw.
32+
const script = `
33+
import { createRequire } from 'node:module';
34+
const require = createRequire(import.meta.url);
35+
const sdk = await import(${JSON.stringify(esmEntryPoint)});
36+
if (typeof sdk.CopilotClient !== 'function') {
37+
process.exit(1);
38+
}
39+
try {
40+
const client = new sdk.CopilotClient({ cliUrl: "8080" });
41+
console.log('CopilotClient constructor: OK');
42+
} catch (e) {
43+
console.error('constructor failed:', e.message);
44+
process.exit(1);
45+
}
46+
`;
47+
const output = execFileSync(
48+
process.execPath,
49+
["--input-type=module", "--eval", script],
50+
{
51+
encoding: "utf-8",
52+
timeout: 10000,
53+
cwd: join(import.meta.dirname, ".."),
54+
},
55+
);
56+
expect(output).toContain("CopilotClient constructor: OK");
57+
});
58+
});

0 commit comments

Comments
 (0)