Skip to content

Commit fb67979

Browse files
darthmolenSteveSandersonMSCopilot
authored
fix(nodejs): add CJS compatibility for VS Code extensions (#546)
* 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. * docs(nodejs): note CJS bundle and system-installed CLI requirements * Dual ESM/CJS build for CommonJS compatibility (#528) Produce both ESM and CJS outputs from the esbuild config so that consumers using either module system get a working package automatically. - Add a second esbuild.build() call with format:"cjs" outputting to dist/cjs/ - Write a dist/cjs/package.json with type:"commonjs" so Node treats .js as CJS - Update package.json exports with "import" and "require" conditions for both the main and ./extension entry points - Revert getBundledCliPath() to use import.meta.resolve for ESM, with a createRequire + path-walking fallback for CJS contexts - Update CJS compatibility tests to verify the actual dual build - Update README to document CJS/CommonJS support Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com> * ci(nodejs): add build step before tests The CJS compatibility tests verify dist/ output, which requires a build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * style(nodejs): fix prettier formatting in changed files Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(nodejs): verify CLI path resolution in both ESM and CJS builds Replace the cliUrl-based test (which skipped getBundledCliPath()) with tests that construct CopilotClient without cliUrl, actually exercising the bundled CLI resolution in both module formats. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * build(nodejs): suppress expected empty-import-meta warning in CJS build The CJS build intentionally produces empty import.meta — our runtime code detects this and falls back to createRequire. Silence the esbuild warning to avoid confusing contributors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Steve Sanderson <SteveSandersonMS@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 2b01b61 commit fb67979

File tree

6 files changed

+158
-11
lines changed

6 files changed

+158
-11
lines changed

.github/workflows/nodejs-sdk-tests.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ jobs:
6262
- name: Typecheck SDK
6363
run: npm run typecheck
6464

65+
- name: Build SDK
66+
run: npm run build
67+
6568
- name: Install test harness dependencies
6669
working-directory: ./test/harness
6770
run: npm ci --ignore-scripts

nodejs/README.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -850,6 +850,26 @@ try {
850850
- Node.js >= 18.0.0
851851
- GitHub Copilot CLI installed and in PATH (or provide custom `cliPath`)
852852

853+
### CJS / CommonJS Support
854+
855+
The SDK ships both ESM and CJS builds. Node.js and bundlers (esbuild, webpack, etc.) automatically select the correct format via the `exports` field in `package.json`:
856+
857+
- `import` / `from` → ESM (`dist/index.js`)
858+
- `require()` → CJS (`dist/cjs/index.cjs`)
859+
860+
This means the SDK works out of the box in CJS environments such as VS Code extensions bundled with `esbuild format:"cjs"`.
861+
862+
### System-installed CLI (winget, brew, apt)
863+
864+
If you installed the Copilot CLI separately rather than relying on the SDK's bundled copy, pass `cliPath` explicitly:
865+
866+
```typescript
867+
const client = new CopilotClient({
868+
cliPath: '/usr/local/bin/copilot', // macOS/Linux
869+
// cliPath: 'C:\\path\\to\\copilot.exe', // Windows (winget, etc.)
870+
});
871+
```
872+
853873
## License
854874

855875
MIT

nodejs/esbuild-copilotsdk-nodejs.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { execSync } from "child_process";
44

55
const entryPoints = globSync("src/**/*.ts");
66

7+
// ESM build
78
await esbuild.build({
89
entryPoints,
910
outbase: "src",
@@ -15,5 +16,22 @@ await esbuild.build({
1516
outExtension: { ".js": ".js" },
1617
});
1718

19+
// CJS build — uses .js extension with a "type":"commonjs" package.json marker
20+
await esbuild.build({
21+
entryPoints,
22+
outbase: "src",
23+
outdir: "dist/cjs",
24+
format: "cjs",
25+
platform: "node",
26+
target: "es2022",
27+
sourcemap: false,
28+
outExtension: { ".js": ".js" },
29+
logOverride: { "empty-import-meta": "silent" },
30+
});
31+
32+
// Mark the CJS directory so Node treats .js files as CommonJS
33+
import { writeFileSync } from "fs";
34+
writeFileSync("dist/cjs/package.json", JSON.stringify({ type: "commonjs" }) + "\n");
35+
1836
// Generate .d.ts files
1937
execSync("tsc", { stdio: "inherit" });

nodejs/package.json

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,28 @@
66
},
77
"version": "0.1.8",
88
"description": "TypeScript SDK for programmatic control of GitHub Copilot CLI via JSON-RPC",
9-
"main": "./dist/index.js",
9+
"main": "./dist/cjs/index.js",
1010
"types": "./dist/index.d.ts",
1111
"exports": {
1212
".": {
13-
"import": "./dist/index.js",
14-
"types": "./dist/index.d.ts"
13+
"import": {
14+
"types": "./dist/index.d.ts",
15+
"default": "./dist/index.js"
16+
},
17+
"require": {
18+
"types": "./dist/index.d.ts",
19+
"default": "./dist/cjs/index.js"
20+
}
1521
},
1622
"./extension": {
17-
"import": "./dist/extension.js",
18-
"types": "./dist/extension.d.ts"
23+
"import": {
24+
"types": "./dist/extension.d.ts",
25+
"default": "./dist/extension.js"
26+
},
27+
"require": {
28+
"types": "./dist/extension.d.ts",
29+
"default": "./dist/cjs/extension.js"
30+
}
1931
}
2032
},
2133
"type": "module",

nodejs/src/client.ts

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
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";
1920
import { fileURLToPath } from "node:url";
@@ -91,14 +92,35 @@ 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+
* In ESM, uses import.meta.resolve directly. In CJS (e.g., VS Code extensions
97+
* bundled with esbuild format:"cjs"), import.meta is empty so we fall back to
98+
* walking node_modules to find the package.
9499
*/
95100
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");
101+
if (typeof import.meta.resolve === "function") {
102+
// ESM: resolve via import.meta.resolve
103+
const sdkUrl = import.meta.resolve("@github/copilot/sdk");
104+
const sdkPath = fileURLToPath(sdkUrl);
105+
// sdkPath is like .../node_modules/@github/copilot/sdk/index.js
106+
// Go up two levels to get the package root, then append index.js
107+
return join(dirname(dirname(sdkPath)), "index.js");
108+
}
109+
110+
// CJS fallback: the @github/copilot package has ESM-only exports so
111+
// require.resolve cannot reach it. Walk the module search paths instead.
112+
const req = createRequire(__filename);
113+
const searchPaths = req.resolve.paths("@github/copilot") ?? [];
114+
for (const base of searchPaths) {
115+
const candidate = join(base, "@github", "copilot", "index.js");
116+
if (existsSync(candidate)) {
117+
return candidate;
118+
}
119+
}
120+
throw new Error(
121+
`Could not find @github/copilot package. Searched ${searchPaths.length} paths. ` +
122+
`Ensure it is installed, or pass cliPath/cliUrl to CopilotClient.`
123+
);
102124
}
103125

104126
/**

nodejs/test/cjs-compat.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/**
2+
* Dual ESM/CJS build compatibility tests
3+
*
4+
* Verifies that both the ESM and CJS builds exist and work correctly,
5+
* so consumers using either module system get a working package.
6+
*
7+
* See: https://github.com/github/copilot-sdk/issues/528
8+
*/
9+
10+
import { describe, expect, it } from "vitest";
11+
import { existsSync } from "node:fs";
12+
import { execFileSync } from "node:child_process";
13+
import { join } from "node:path";
14+
15+
const distDir = join(import.meta.dirname, "../dist");
16+
17+
describe("Dual ESM/CJS build (#528)", () => {
18+
it("ESM dist file should exist", () => {
19+
expect(existsSync(join(distDir, "index.js"))).toBe(true);
20+
});
21+
22+
it("CJS dist file should exist", () => {
23+
expect(existsSync(join(distDir, "cjs/index.js"))).toBe(true);
24+
});
25+
26+
it("CJS build is requireable and exports CopilotClient", () => {
27+
const script = `
28+
const sdk = require(${JSON.stringify(join(distDir, "cjs/index.js"))});
29+
if (typeof sdk.CopilotClient !== 'function') {
30+
console.error('CopilotClient is not a function');
31+
process.exit(1);
32+
}
33+
console.log('CJS require: OK');
34+
`;
35+
const output = execFileSync(process.execPath, ["--eval", script], {
36+
encoding: "utf-8",
37+
timeout: 10000,
38+
cwd: join(import.meta.dirname, ".."),
39+
});
40+
expect(output).toContain("CJS require: OK");
41+
});
42+
43+
it("CJS build resolves bundled CLI path", () => {
44+
const script = `
45+
const sdk = require(${JSON.stringify(join(distDir, "cjs/index.js"))});
46+
const client = new sdk.CopilotClient({ autoStart: false });
47+
console.log('CJS CLI resolved: OK');
48+
`;
49+
const output = execFileSync(process.execPath, ["--eval", script], {
50+
encoding: "utf-8",
51+
timeout: 10000,
52+
cwd: join(import.meta.dirname, ".."),
53+
});
54+
expect(output).toContain("CJS CLI resolved: OK");
55+
});
56+
57+
it("ESM build resolves bundled CLI path", () => {
58+
const esmPath = join(distDir, "index.js");
59+
const script = `
60+
import { pathToFileURL } from 'node:url';
61+
const sdk = await import(pathToFileURL(${JSON.stringify(esmPath)}).href);
62+
const client = new sdk.CopilotClient({ autoStart: false });
63+
console.log('ESM CLI resolved: OK');
64+
`;
65+
const output = execFileSync(process.execPath, ["--input-type=module", "--eval", script], {
66+
encoding: "utf-8",
67+
timeout: 10000,
68+
cwd: join(import.meta.dirname, ".."),
69+
});
70+
expect(output).toContain("ESM CLI resolved: OK");
71+
});
72+
});

0 commit comments

Comments
 (0)