From 4655e15d6f27db9c38d8a51f59c89d6dcae4e250 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sun, 4 Oct 2026 01:19:10 -0700 Subject: [PATCH] fix(extensions): resolve folder-extension node_modules imports in compiled binaries After the host-specifier rewrite, onLoad runs rewriteInstalledPackageSpecifiers on the transpiled source of every file under the extension directory, including packages installed inside it. Each static bare specifier in import, export-from, import(), or require() is replaced with the file installed for it: a file URL for the module forms, and an escaped absolute path for require(). Resolution uses Bun.resolveSync when that path exists on disk, then createRequire from the importer, then the package root import or default export when require cannot read an import-only exports map. Relative paths, Node and Bun builtins, hunk-host and file URLs, and packages neither resolver can find stay as written, so a missing package still fails with the existing error. Fixes #1136 --- .changeset/compiled-extension-imports.md | 5 + .../src/extensions/hostRuntimeModules.test.ts | 194 +++++++++++++++- .../hunk/src/extensions/hostRuntimeModules.ts | 215 +++++++++++++++++- 3 files changed, 408 insertions(+), 6 deletions(-) create mode 100644 .changeset/compiled-extension-imports.md diff --git a/.changeset/compiled-extension-imports.md b/.changeset/compiled-extension-imports.md new file mode 100644 index 000000000..cc4535f71 --- /dev/null +++ b/.changeset/compiled-extension-imports.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Folder extensions load packages installed in their own node_modules when Hunk runs as a compiled binary. diff --git a/packages/hunk/src/extensions/hostRuntimeModules.test.ts b/packages/hunk/src/extensions/hostRuntimeModules.test.ts index 985a5e461..a3f56974f 100644 --- a/packages/hunk/src/extensions/hostRuntimeModules.test.ts +++ b/packages/hunk/src/extensions/hostRuntimeModules.test.ts @@ -6,7 +6,10 @@ import { afterEach, describe, expect, test } from "bun:test"; import { TextAttributes } from "@opentui/core"; import { isValidElement, useState } from "react"; import { HunkExtensionUserError } from "../extension-api"; -import { registerHostRuntimeModules } from "./hostRuntimeModules"; +import { + registerHostRuntimeModules, + rewriteInstalledPackageSpecifiers, +} from "./hostRuntimeModules"; /** * These tests import real files from a temp directory, the way extension @@ -178,4 +181,193 @@ describe("registerHostRuntimeModules", () => { await expect(import(pathToFileURL(outsider).href)).rejects.toThrow(/react/); }); + + test("loads a package from the extension's own node_modules", async () => { + const root = mkdtempSync(join(tmpdir(), "hunk-host-modules-dep-")); + tempDirs.push(root); + const entryDir = join(root, "src"); + mkdirSync(entryDir); + const path = join(entryDir, "index.ts"); + writeFileSync(path, `import fakeDep from "fake-dep";\nexport default fakeDep;\n`); + const depDir = join(root, "node_modules", "fake-dep"); + mkdirSync(depDir, { recursive: true }); + writeFileSync( + join(depDir, "package.json"), + JSON.stringify({ name: "fake-dep", version: "1.0.0", main: "index.js" }), + ); + writeFileSync(join(depDir, "index.js"), `module.exports = { language: "graphql" };\n`); + + const mod = await importTempExtension(path); + + expect(mod.default).toEqual({ language: "graphql" }); + }); + + test("does not resolve a type-only import the transpiler erases", async () => { + const path = writeTempExtension( + "ext.ts", + `import type { Missing } from "type-only-pkg";\nexport default { ready: true as Missing | boolean };\n`, + ); + + const mod = await importTempExtension(path); + + expect(mod.default.ready).toBe(true); + }); + + test("leaves an unknown package bare and the import rejects", async () => { + const path = writeTempExtension("ext.ts", `import "not-a-real-package";\nexport default {};\n`); + expect(rewriteInstalledPackageSpecifiers(`import "not-a-real-package";\n`, path)).toBe( + `import "not-a-real-package";\n`, + ); + + await expect(importTempExtension(path)).rejects.toThrow(/not-a-real-package/); + }); +}); + +describe("rewriteInstalledPackageSpecifiers", () => { + test("substitutes the file Bun resolves for an extension and for a dependency", () => { + const root = mkdtempSync(join(tmpdir(), "hunk-host-modules-rewrite-")); + tempDirs.push(root); + const entry = join(root, "index.ts"); + writeFileSync(entry, `export {}\n`); + const fakeDep = join(root, "node_modules", "fake-dep"); + mkdirSync(fakeDep, { recursive: true }); + writeFileSync( + join(fakeDep, "package.json"), + JSON.stringify({ name: "fake-dep", main: "index.js" }), + ); + writeFileSync(join(fakeDep, "index.js"), `module.exports = { language: "graphql" };\n`); + const depA = join(root, "node_modules", "dep-a"); + const depB = join(root, "node_modules", "dep-b"); + mkdirSync(depA, { recursive: true }); + mkdirSync(depB, { recursive: true }); + writeFileSync(join(depA, "package.json"), JSON.stringify({ name: "dep-a", main: "index.js" })); + writeFileSync(join(depA, "index.js"), `import value from "dep-b";\nexport default value;\n`); + writeFileSync(join(depB, "package.json"), JSON.stringify({ name: "dep-b", main: "index.js" })); + writeFileSync(join(depB, "index.js"), `module.exports = { ok: true };\n`); + + const resolvedDep = Bun.resolveSync("fake-dep", entry); + const fromExtension = rewriteInstalledPackageSpecifiers( + `export { language } from "fake-dep";\n`, + entry, + ); + expect(fromExtension).toContain(pathToFileURL(resolvedDep).href); + expect(fromExtension).not.toContain(`"fake-dep"`); + + const requireRewrite = rewriteInstalledPackageSpecifiers( + `const dep = require("fake-dep");\n`, + entry, + ); + // require() keeps the absolute path, and the rewrite doubles backslashes + // so the specifier stays valid inside the original quotes. + expect(requireRewrite).toContain(resolvedDep.replaceAll("\\", "\\\\").replaceAll('"', '\\"')); + expect(requireRewrite).not.toContain("file:"); + + const depAEntry = join(depA, "index.js"); + const resolvedSibling = Bun.resolveSync("dep-b", depAEntry); + const fromDependency = rewriteInstalledPackageSpecifiers( + `import value from "dep-b";\n`, + depAEntry, + ); + expect(fromDependency).toContain(pathToFileURL(resolvedSibling).href); + }); + + test("rewrites an import-only export when Bun cannot resolve the package", () => { + const root = mkdtempSync(join(tmpdir(), "hunk-host-modules-esm-")); + tempDirs.push(root); + const entry = join(root, "index.ts"); + writeFileSync(entry, `export {}\n`); + const pkgDir = join(root, "node_modules", "esm-only"); + mkdirSync(pkgDir, { recursive: true }); + writeFileSync( + join(pkgDir, "package.json"), + JSON.stringify({ + name: "esm-only", + exports: { ".": { import: "./entry.js" } }, + }), + ); + writeFileSync(join(pkgDir, "entry.js"), `export const language = "md";\n`); + + const original = Bun.resolveSync; + Bun.resolveSync = (() => { + throw Object.assign(new Error("Cannot find package 'esm-only'"), { + code: "ERR_MODULE_NOT_FOUND", + }); + }) as typeof Bun.resolveSync; + try { + const rewritten = rewriteInstalledPackageSpecifiers(`import pkg from "esm-only";\n`, entry); + expect(rewritten).toContain(pathToFileURL(join(pkgDir, "entry.js")).href); + } finally { + Bun.resolveSync = original; + } + }); + + test("rewrites a root-level import condition when exports has no dot key", () => { + const root = mkdtempSync(join(tmpdir(), "hunk-host-modules-esm-root-")); + tempDirs.push(root); + const entry = join(root, "index.ts"); + writeFileSync(entry, `export {}\n`); + const pkgDir = join(root, "node_modules", "esm-root"); + mkdirSync(pkgDir, { recursive: true }); + writeFileSync( + join(pkgDir, "package.json"), + JSON.stringify({ + name: "esm-root", + exports: { import: "./entry.js" }, + }), + ); + writeFileSync(join(pkgDir, "entry.js"), `export const language = "md";\n`); + + const original = Bun.resolveSync; + Bun.resolveSync = (() => { + throw Object.assign(new Error("Cannot find package 'esm-root'"), { + code: "ERR_MODULE_NOT_FOUND", + }); + }) as typeof Bun.resolveSync; + try { + const rewritten = rewriteInstalledPackageSpecifiers(`import pkg from "esm-root";\n`, entry); + expect(rewritten).toContain(pathToFileURL(join(pkgDir, "entry.js")).href); + } finally { + Bun.resolveSync = original; + } + }); + + test("keeps relative, node, and bun specifiers unchanged", () => { + const path = writeTempExtension("ext.ts", `export default {};\n`); + const source = [ + `import helper from "./helper";`, + `import marked from "./node_modules/marked/lib/marked.esm.js";`, + `import fs from "node:fs";`, + `import bareFs from "fs";`, + `import promises from "fs/promises";`, + `import ffi from "bun:ffi";`, + ].join("\n"); + + expect(rewriteInstalledPackageSpecifiers(source, path)).toBe(source); + }); + + test("rewrites scoped package names and subpaths", () => { + const root = mkdtempSync(join(tmpdir(), "hunk-host-modules-scope-")); + tempDirs.push(root); + const entry = join(root, "index.ts"); + writeFileSync(entry, `export {}\n`); + const scoped = join(root, "node_modules", "@scope", "pkg"); + mkdirSync(scoped, { recursive: true }); + writeFileSync( + join(scoped, "package.json"), + JSON.stringify({ name: "@scope/pkg", main: "index.js" }), + ); + writeFileSync(join(scoped, "index.js"), `module.exports = { name: "scoped" };\n`); + const subpath = join(root, "node_modules", "pkg"); + mkdirSync(subpath, { recursive: true }); + writeFileSync(join(subpath, "package.json"), JSON.stringify({ name: "pkg", main: "index.js" })); + writeFileSync(join(subpath, "entry.js"), `module.exports = { sub: true };\n`); + + const rewritten = rewriteInstalledPackageSpecifiers( + `import scoped from "@scope/pkg";\nimport entry from "pkg/entry";\n`, + entry, + ); + + expect(rewritten).toContain(pathToFileURL(Bun.resolveSync("@scope/pkg", entry)).href); + expect(rewritten).toContain(pathToFileURL(Bun.resolveSync("pkg/entry", entry)).href); + }); }); diff --git a/packages/hunk/src/extensions/hostRuntimeModules.ts b/packages/hunk/src/extensions/hostRuntimeModules.ts index cc175bd07..3c7cb87ac 100644 --- a/packages/hunk/src/extensions/hostRuntimeModules.ts +++ b/packages/hunk/src/extensions/hostRuntimeModules.ts @@ -1,4 +1,7 @@ -import { dirname } from "node:path"; +import { existsSync, readFileSync } from "node:fs"; +import { createRequire, isBuiltin } from "node:module"; +import { dirname, isAbsolute, join, resolve } from "node:path"; +import { pathToFileURL } from "node:url"; import { resolveCanonicalPath } from "../core/run/paths"; /** @@ -25,6 +28,11 @@ import { resolveCanonicalPath } from "../core/run/paths"; * `@opentui/react/jsx-runtime` under a pragma), and they only pass through the * rewrite if they exist before Bun's own loader would have added them. * + * The same returned source has to name files for packages the extension + * installed beside itself. A compiled binary does not resolve those bare + * specifiers from the extension file, so after the host rewrite each static + * bare specifier becomes the absolute file its own `node_modules` provides. + * * Modules linked into the compiled binary never re-resolve their imports, so * none of this affects the host bundle in any run mode. */ @@ -81,6 +89,196 @@ export function rewriteHostSpecifiers(code: string) { ); } +/** + * Match one static specifier in import position in transpiled output. + * + * Same positions as the host rewrite: `from "x"`, `import("x")`, and + * `require("x")`. Relative paths, builtins, and `hunk-host:` modules are + * filtered after the match so a data string is still left alone. + */ +const INSTALLED_PACKAGE_SPECIFIER_PATTERN = + /((?:\bfrom|\bimport|\brequire)\s*\(?\s*)(["'])([^"']+)\2/g; + +/** + * Replace static bare package specifiers with the file installed for them. + * + * `Bun.resolveSync` is enough when it returns a file on disk. A compiled + * binary still reports the extension file as the importer and cannot see that + * file, so `createRequire` walks the extension directory's real `node_modules`. + * An import-only `exports` map throws instead of resolving: Node uses + * `ERR_PACKAGE_PATH_NOT_EXPORTED`, and Bun's `createRequire` uses + * `MODULE_NOT_FOUND`. The nearest package.json then supplies the package root: + * `exports["."]` when the map lists subpaths, otherwise an `import` or + * `default` string on the map itself. Pattern targets are skipped. A specifier + * neither resolver can find stays bare, so the existing "Cannot find package" + * error still surfaces. Bare Node builtins resolve to a module id, not a file, + * and those specifiers stay as written. + * + * `import`, `export`, and `import()` receive a file URL of an absolute path. + * `require()` receives the raw absolute path, with quotes and backslashes + * escaped for the quotes already in the source. + */ +export function rewriteInstalledPackageSpecifiers(code: string, importerPath: string) { + return code.replace( + INSTALLED_PACKAGE_SPECIFIER_PATTERN, + (all, lead: string, quote: string, specifier: string) => { + if (isPreservedSpecifier(specifier)) return all; + const resolved = resolveInstalledPackage(specifier, importerPath); + // A builtin id such as "fs" is not a file. pathToFileURL would rewrite + // import/export/import() to a cwd-relative file URL. + if (!resolved || !isAbsolute(resolved) || isBuiltin(resolved)) return all; + const value = /\brequire\b/.test(lead) ? resolved : pathToFileURL(resolved).href; + return `${lead}${quote}${escapeQuotedSpecifier(value, quote)}${quote}`; + }, + ); +} + +/** Keep specifiers the runtime or the host rewrite already owns. */ +function isPreservedSpecifier(specifier: string) { + return ( + specifier.startsWith(".") || + specifier.startsWith("node:") || + specifier.startsWith("bun:") || + specifier.startsWith("hunk-host:") || + specifier.startsWith("file:") || + isAbsolute(specifier) || + isBuiltin(specifier) + ); +} + +/** Escape one resolved path so it remains a single quoted specifier. */ +function escapeQuotedSpecifier(value: string, quote: string) { + const escaped = value.replace(/\\/g, "\\\\"); + return escaped.replaceAll(quote, `\\${quote}`); +} + +/** Resolve one bare specifier from the file that imported it, or nothing. */ +function resolveInstalledPackage(specifier: string, importerPath: string) { + const bunResolved = resolveWithBun(specifier, importerPath); + if (bunResolved) return bunResolved; + + try { + return createRequire(importerPath).resolve(specifier); + } catch (error) { + if (!isUnexportedPackageError(error)) return undefined; + return resolveUnexportedPackageEntry(specifier, importerPath); + } +} + +/** Use Bun's resolver only when the file it names is actually on disk. */ +function resolveWithBun(specifier: string, importerPath: string) { + try { + const resolved = Bun.resolveSync(specifier, importerPath); + if (typeof resolved === "string" && existsSync(resolved)) return resolved; + } catch { + // The compiled binary names this importer and still cannot see its + // node_modules. createRequire walks that directory for real. + } + return undefined; +} + +/** + * Report whether require() could not export the package. + * + * Node throws `ERR_PACKAGE_PATH_NOT_EXPORTED` when `exports` has no require + * target. Bun's `createRequire` throws `MODULE_NOT_FOUND` for that map, and + * also for a package that is not installed. The export fallback then finds a + * package.json only in the first case; otherwise the specifier stays bare. + */ +function isUnexportedPackageError(error: unknown) { + if (typeof error !== "object" || error === null || !("code" in error)) return false; + const code = (error as { code?: unknown }).code; + return code === "ERR_PACKAGE_PATH_NOT_EXPORTED" || code === "MODULE_NOT_FOUND"; +} + +/** + * Read the package root file from `exports` when require() cannot. + * + * Only the package name itself uses `exports["."]`. A subpath that failed to + * resolve stays bare rather than being rewritten to the package root. + */ +function resolveUnexportedPackageEntry(specifier: string, importerPath: string) { + const packageName = packageNameFromSpecifier(specifier); + if (!packageName || specifier !== packageName) return undefined; + + const packageJsonPath = nearestPackageJson(importerPath, packageName); + if (!packageJsonPath) return undefined; + + let manifest: { exports?: unknown }; + try { + manifest = JSON.parse(readFileSync(packageJsonPath, "utf8")) as { exports?: unknown }; + } catch { + return undefined; + } + + const relativeTarget = relativePackageEntry(manifest.exports); + if (!relativeTarget) return undefined; + const absolute = resolve(dirname(packageJsonPath), relativeTarget); + return existsSync(absolute) ? absolute : undefined; +} + +/** Package name of a bare specifier, keeping the scope when there is one. */ +function packageNameFromSpecifier(specifier: string) { + if (specifier.startsWith("@")) { + const [scope, name] = specifier.split("/"); + if (!scope || !name) return undefined; + return `${scope}/${name}`; + } + const name = specifier.split("/")[0]; + return name || undefined; +} + +/** Nearest `node_modules//package.json` above the importing file. */ +function nearestPackageJson(importerPath: string, packageName: string) { + const segments = packageName.split("/"); + let directory = dirname(importerPath); + for (;;) { + const candidate = join(directory, "node_modules", ...segments, "package.json"); + if (existsSync(candidate)) return candidate; + const parent = dirname(directory); + if (parent === directory) return undefined; + directory = parent; + } +} + +/** + * Relative file named by the package root export. + * + * A string export is that file. When the map lists subpaths, `exports["."]` + * is the root. Otherwise `import` or `default` sits on the map itself. + * Targets that contain `*` are patterns, not one file, and are skipped. + */ +function relativePackageEntry(exportsField: unknown) { + if (typeof exportsField !== "object" || exportsField === null || Array.isArray(exportsField)) { + return relativeExportTarget(exportsField); + } + + const record = exportsField as Record; + const root = "." in record ? record["."] : record; + return relativeExportTarget(root); +} + +/** Pick the relative file from one export target or its import/default condition. */ +function relativeExportTarget(value: unknown): string | undefined { + if (typeof value === "string") return relativeFileTarget(value); + if (typeof value !== "object" || value === null || Array.isArray(value)) return undefined; + + const conditions = value as Record; + for (const key of ["import", "default"] as const) { + const target = conditions[key]; + if (typeof target !== "string") continue; + const relative = relativeFileTarget(target); + if (relative) return relative; + } + return undefined; +} + +/** Accept one relative file target and reject export patterns. */ +function relativeFileTarget(target: string) { + if (!target.startsWith(".") || target.includes("*")) return undefined; + return target; +} + type TranspilerLoader = "js" | "jsx" | "ts" | "tsx"; /** Pick the transpiler loader for one extension file path. */ @@ -171,9 +369,10 @@ function registerSourceRoot(directory: string) { } registeredSourceRoots.add(sourceRoot); - // Everything under the directory, so a folder extension's helper modules get - // the same rewrite as its entry file. `[/\\]` keeps the boundary correct on - // Windows, where `args.path` carries native separators. + // Everything under the directory, so a folder extension's helper modules and + // the files of packages installed inside it get the same rewrite as its + // entry. `[/\\]` keeps the boundary correct on Windows, where `args.path` + // carries native separators. const escapedDirectory = sourceRoot.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); const filter = new RegExp(`^${escapedDirectory}[/\\\\].*\\.(?:[mc]?[jt]s|[jt]sx)$`); @@ -183,7 +382,13 @@ function registerSourceRoot(directory: string) { build.onLoad({ filter }, async (args) => { const source = await Bun.file(args.path).text(); const transpiled = transpilerFor(resolveLoader(args.path)).transformSync(source); - return { contents: rewriteHostSpecifiers(transpiled), loader: "js" }; + return { + contents: rewriteInstalledPackageSpecifiers( + rewriteHostSpecifiers(transpiled), + args.path, + ), + loader: "js", + }; }); }, });