Skip to content

Commit fb560ad

Browse files
committed
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 f0909a7 commit fb560ad

File tree

2 files changed

+81
-7
lines changed

2 files changed

+81
-7
lines changed

nodejs/src/client.ts

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@
1313

1414
import { spawn, type ChildProcess } from "node:child_process";
1515
import { existsSync } from "node:fs";
16+
import { createRequire } from "node:module";
1617
import { Socket } from "node:net";
1718
import { dirname, join } from "node:path";
18-
import { fileURLToPath } from "node:url";
19+
import { pathToFileURL } from "node:url";
1920
import {
2021
createMessageConnection,
2122
MessageConnection,
@@ -117,14 +118,29 @@ function getNodeExecPath(): string {
117118
/**
118119
* Gets the path to the bundled CLI from the @github/copilot package.
119120
* Uses index.js directly rather than npm-loader.js (which spawns the native binary).
121+
*
122+
* The @github/copilot package only exposes an ESM-only "./sdk" export,
123+
* which breaks in CJS contexts (e.g., VS Code extensions bundled with esbuild).
124+
* Instead of resolving through the package's exports, we locate the package
125+
* root by walking module resolution paths and checking for its directory.
126+
* See: https://github.com/github/copilot-sdk/issues/528
120127
*/
121128
function getBundledCliPath(): string {
122-
// Find the actual location of the @github/copilot package by resolving its sdk export
123-
const sdkUrl = import.meta.resolve("@github/copilot/sdk");
124-
const sdkPath = fileURLToPath(sdkUrl);
125-
// sdkPath is like .../node_modules/@github/copilot/sdk/index.js
126-
// Go up two levels to get the package root, then append index.js
127-
return join(dirname(dirname(sdkPath)), "index.js");
129+
// import.meta.url is defined in ESM; in CJS bundles (esbuild format:"cjs")
130+
// it's undefined, so we fall back to __filename via pathToFileURL.
131+
const require = createRequire(import.meta.url ?? pathToFileURL(__filename).href);
132+
// The @github/copilot package has strict ESM-only exports, so require.resolve
133+
// cannot resolve it. Instead, walk the module resolution paths to find it.
134+
const searchPaths = require.resolve.paths("@github/copilot") ?? [];
135+
for (const base of searchPaths) {
136+
const candidate = join(base, "@github", "copilot", "index.js");
137+
if (existsSync(candidate)) {
138+
return candidate;
139+
}
140+
}
141+
throw new Error(
142+
`Could not find @github/copilot package. Searched ${searchPaths.length} paths. Ensure it is installed.`
143+
);
128144
}
129145

130146
export class CopilotClient {

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)