Skip to content

Commit 3b6d5d4

Browse files
anandgupta42claude
anandgupta42
and
claude
authored
fix: dbt commands fail with ESM SyntaxError since v0.5.3 (#407)
* fix: dbt commands fail with `SyntaxError: Cannot use import statement` on Node The `publish.ts` `copyAssets()` function copies `dbt-tools/bin/` and `dbt-tools/dist/` but omits `package.json`. Without `"type": "module"`, Node defaults to CJS and rejects the ESM `import` statements in `dist/index.js`. Bun never surfaced this because it handles ESM natively. Synthesize a minimal `{"type":"module"}` `package.json` into the bundled `dbt-tools/` directory. Add a dedicated test file (`dbt-tools-bundle.test.ts`) that guards the full ESM chain: source contract, publish bundling, and the Node+ESM regression scenario. Broken since v0.5.3 (#316). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address code review — idempotency guard and test assertion accuracy - Add `if (!pkg.name || !pkg.version) continue` guard in publish.ts glob loop to prevent crash on re-run when synthesized package.json (lacking name/version) is picked up - Fix test assertion to match JS object literal `type: "module"` instead of the comment containing `"type": "module"` Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: adversarial e2e tests for dbt-tools ESM loading under Node 14 tests covering all installation scenarios: - Symlink path (default postinstall) - Wrapper path (symlink fallback) - Direct invocation (Windows .cmd shim) - Missing/wrong `package.json` (reproduces original bug) - Bun runtime (verify fix doesn't break Bun users) - `package.json` hierarchy edge cases Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address PR review — strengthen test assertions per CodeRabbit - Match actual `Bun.file().write()` + `JSON.stringify()` calls instead of raw strings that could match comments - Remove conditional `if (usesNode && usesESMImport)` — assert prerequisites explicitly so the regression guard cannot pass vacuously Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 6b7070d commit 3b6d5d4

File tree

5 files changed

+671
-0
lines changed

5 files changed

+671
-0
lines changed

‎packages/opencode/script/publish.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ const driverPeerDependenciesMeta: Record<string, { optional: true }> = Object.fr
3838
const binaries: Record<string, string> = {}
3939
for (const filepath of new Bun.Glob("**/package.json").scanSync({ cwd: "./dist" })) {
4040
const pkg = await Bun.file(`./dist/${filepath}`).json()
41+
// Skip synthesized package.json files (e.g. dbt-tools) that lack name/version
42+
if (!pkg.name || !pkg.version) continue
4143
binaries[pkg.name] = pkg.version
4244
}
4345
console.log("binaries", binaries)
@@ -63,6 +65,13 @@ async function copyAssets(targetDir: string) {
6365
await $`cp ../dbt-tools/bin/altimate-dbt ${targetDir}/dbt-tools/bin/altimate-dbt`
6466
await $`mkdir -p ${targetDir}/dbt-tools/dist`
6567
await $`cp ../dbt-tools/dist/index.js ${targetDir}/dbt-tools/dist/`
68+
// A package.json with "type": "module" must be present so Node loads
69+
// dist/index.js as ESM instead of CJS. We synthesize a minimal one rather
70+
// than copying the full source package.json (which contains devDependencies
71+
// with Bun catalog: versions that would confuse vulnerability scanners).
72+
await Bun.file(`${targetDir}/dbt-tools/package.json`).write(
73+
JSON.stringify({ type: "module" }, null, 2) + "\n",
74+
)
6675
if (fs.existsSync("../dbt-tools/dist/altimate_python_packages")) {
6776
await $`cp -r ../dbt-tools/dist/altimate_python_packages ${targetDir}/dbt-tools/dist/`
6877
}

‎packages/opencode/test/branding/build-integrity.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,8 @@ describe("Bundle Completeness", () => {
295295
expect(publishScript).toContain("dbt-tools/bin/altimate-dbt")
296296
expect(publishScript).toContain("dbt-tools/dist")
297297
expect(publishScript).toContain("bun run build")
298+
// package.json must be bundled so Node sees "type": "module"
299+
expect(publishScript).toContain("dbt-tools/package.json")
298300
})
299301

300302
test("publish.ts copies only needed dbt-tools dist files (not .node binaries)", () => {

‎packages/opencode/test/install/dbt-tools-bundle.test.ts‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
import { describe, test, expect } from "bun:test"
2+
import fs from "fs"
3+
import path from "path"
4+
5+
const REPO_ROOT = path.resolve(import.meta.dir, "../../../..")
6+
const PUBLISH_SCRIPT = path.join(REPO_ROOT, "packages/opencode/script/publish.ts")
7+
const DBT_TOOLS_DIR = path.join(REPO_ROOT, "packages/dbt-tools")
8+
9+
// ---------------------------------------------------------------------------
10+
// 1. Source dbt-tools uses ESM — if this changes, publish.ts must adapt
11+
// ---------------------------------------------------------------------------
12+
13+
describe("dbt-tools ESM contract", () => {
14+
test('source package.json declares "type": "module"', () => {
15+
const pkg = JSON.parse(fs.readFileSync(path.join(DBT_TOOLS_DIR, "package.json"), "utf-8"))
16+
expect(pkg.type).toBe("module")
17+
})
18+
19+
test("bin/altimate-dbt uses node shebang (not bun)", () => {
20+
const bin = fs.readFileSync(path.join(DBT_TOOLS_DIR, "bin/altimate-dbt"), "utf-8")
21+
expect(bin).toContain("#!/usr/bin/env node")
22+
})
23+
24+
test("bin/altimate-dbt uses ESM import() to load dist/index.js", () => {
25+
const bin = fs.readFileSync(path.join(DBT_TOOLS_DIR, "bin/altimate-dbt"), "utf-8")
26+
expect(bin).toContain('import("../dist/index.js")')
27+
})
28+
29+
test("build outputs ESM format (--format esm in build script)", () => {
30+
const pkg = JSON.parse(fs.readFileSync(path.join(DBT_TOOLS_DIR, "package.json"), "utf-8"))
31+
const buildScript = pkg.scripts?.build || ""
32+
expect(buildScript).toContain("--format esm")
33+
})
34+
})
35+
36+
// ---------------------------------------------------------------------------
37+
// 2. publish.ts must bundle a package.json with "type": "module"
38+
// ---------------------------------------------------------------------------
39+
40+
describe("publish.ts dbt-tools ESM bundling", () => {
41+
const publishSource = fs.readFileSync(PUBLISH_SCRIPT, "utf-8")
42+
43+
test("copyAssets writes a dbt-tools/package.json", () => {
44+
// The publish script must create a package.json in the bundled dbt-tools dir.
45+
// Without it, Node defaults to CJS and `import` statements in dist/index.js fail.
46+
expect(publishSource).toContain("dbt-tools/package.json")
47+
})
48+
49+
test('copyAssets writes package.json with type: "module" via Bun.file', () => {
50+
// Match the actual write call, not just a comment or string literal.
51+
expect(publishSource).toContain('Bun.file(`${targetDir}/dbt-tools/package.json`).write')
52+
expect(publishSource).toContain('JSON.stringify({ type: "module" }')
53+
})
54+
55+
test("copyAssets creates dbt-tools/bin and dbt-tools/dist directories", () => {
56+
expect(publishSource).toContain("dbt-tools/bin")
57+
expect(publishSource).toContain("dbt-tools/dist")
58+
})
59+
60+
test("copyAssets copies the altimate-dbt bin wrapper", () => {
61+
expect(publishSource).toContain("dbt-tools/bin/altimate-dbt")
62+
})
63+
64+
test("copyAssets copies dist/index.js (not the entire dist/ tree)", () => {
65+
// Copying all of dist/ would include ~220MB of .node native binaries
66+
expect(publishSource).toContain("dist/index.js")
67+
})
68+
})
69+
70+
// ---------------------------------------------------------------------------
71+
// 3. Structural: if dbt-tools ever switches away from ESM, tests should catch it
72+
// ---------------------------------------------------------------------------
73+
74+
describe("dbt-tools + Node compatibility", () => {
75+
test("bin wrapper import path matches dist output location", () => {
76+
// The bin wrapper does `import("../dist/index.js")`.
77+
// If the build output location changes, this test forces an update.
78+
const bin = fs.readFileSync(path.join(DBT_TOOLS_DIR, "bin/altimate-dbt"), "utf-8")
79+
const match = bin.match(/import\(["']([^"']+)["']\)/)
80+
expect(match).not.toBeNull()
81+
const importPath = match![1]
82+
expect(importPath).toBe("../dist/index.js")
83+
})
84+
85+
test("Node would fail without package.json type:module (regression guard)", () => {
86+
// This is the exact scenario that caused the bug:
87+
// - bin/altimate-dbt has `#!/usr/bin/env node`
88+
// - It uses `import("../dist/index.js")`
89+
// - dist/index.js starts with `import { createRequire } from "node:module"`
90+
// - Without "type": "module" in package.json, Node treats .js as CJS
91+
// - CJS cannot use top-level `import` → SyntaxError
92+
//
93+
// This test verifies the chain of conditions that require package.json:
94+
const bin = fs.readFileSync(path.join(DBT_TOOLS_DIR, "bin/altimate-dbt"), "utf-8")
95+
96+
// Assert prerequisites explicitly so the test cannot pass vacuously
97+
const usesNode = bin.includes("#!/usr/bin/env node")
98+
const usesESMImport = bin.includes("import(")
99+
expect(usesNode).toBe(true)
100+
expect(usesESMImport).toBe(true)
101+
102+
// Given both conditions hold, package.json MUST have "type": "module"
103+
const pkg = JSON.parse(fs.readFileSync(path.join(DBT_TOOLS_DIR, "package.json"), "utf-8"))
104+
expect(pkg.type).toBe("module")
105+
106+
// AND publish.ts MUST bundle that information via Bun.file().write()
107+
const publishSource = fs.readFileSync(PUBLISH_SCRIPT, "utf-8")
108+
expect(publishSource).toContain('JSON.stringify({ type: "module" }')
109+
})
110+
111+
test("all dbt-tools bin entries use node shebang (consistency check)", () => {
112+
const pkg = JSON.parse(fs.readFileSync(path.join(DBT_TOOLS_DIR, "package.json"), "utf-8"))
113+
const binEntries: Record<string, string> = pkg.bin || {}
114+
115+
for (const [name, relPath] of Object.entries(binEntries)) {
116+
const binPath = path.join(DBT_TOOLS_DIR, relPath)
117+
expect(fs.existsSync(binPath)).toBe(true)
118+
119+
const content = fs.readFileSync(binPath, "utf-8")
120+
// All bin entries should use node (not bun) since end users may not have bun
121+
expect(content).toContain("#!/usr/bin/env node")
122+
}
123+
})
124+
})

0 commit comments

Comments
 (0)