Skip to content

Importing one name from a module also binds that module's OTHER exported classes — a user class Request shadows the global Request in the importer (OpenCode TUI bootstrap wall) #10356

Description

@proggeramlug

Summary

A module that imports anything from another module also picks up every exported class of that module as a bare identifier — including names that are global intrinsics. So a user module exporting class Request makes a completely unrelated new Request(url, init) in the importing module construct that user class instead of the global fetch Request.

This is the current OpenCode TUI bootstrap failure. packages/sdk/js/src/v2/client.ts does:

import { OpencodeClient } from "./gen/sdk.gen.js"   // ← imports ONE name
...
function rewrite(request: Request, values: {...}) {
  const url = new URL(request.url)
  ...
  const next = new Request(url, request)            // ← must be the GLOBAL Request
  next.headers.delete("x-opencode-directory")       // ← TypeError here
}

gen/sdk.gen.ts:6319 happens to contain export class Request extends HeyApiClient. It is not in the specifier list, so per ESM it is not in client.ts's scope at all. perry binds it anyway, next is a HeyApiClient, next.headers is undefined, and the TUI dies with:

tui bootstrap failed { error: "Cannot read properties of undefined (reading 'delete')" }

Reproducer

mod.ts:

export class HeyApiClient {
  client: any
  constructor(config: any) { this.client = config?.client ?? null }
}
export class Request extends HeyApiClient {
  readonly kind = "user-sdk-request"
}
export class OpencodeClient {
  readonly name = "OpencodeClient"
}

main.ts:

// We import ONLY OpencodeClient. Per ESM, `Request` below must be the GLOBAL
// fetch Request -- a named import binds exactly the names it lists.
import { OpencodeClient } from "./mod.js"

const c = new OpencodeClient()
console.log("1 import-works:", c.name)

const url = new URL("http://example.com/x?a=1")
const base = new Request(url, { method: "GET" })
console.log("2 base.method:", base.method)
console.log("3 base.url:", base.url)
console.log("4 typeof base.headers:", typeof base.headers)

const next = new Request(url, base)
console.log("5 next.method:", next.method)
console.log("6 typeof next.headers:", typeof next.headers)
try {
  next.headers.delete("x-opencode-directory")
  console.log("7 headers.delete:", "ok")
} catch (e: any) {
  console.log("7 headers.delete:", "THREW " + e.message)
}
console.log("8 instanceof global Request:", base instanceof Request)
console.log("9 kind-leak:", (base as any).kind)

Measured

# cell bun 1.3.14 perry
1 new OpencodeClient().name OpencodeClient OpencodeClient ok
2 base.method GET undefined
3 base.url http://example.com/x?a=1 undefined
4 typeof base.headers object undefined
5 next.method GET undefined
6 typeof next.headers object undefined
7 next.headers.delete(...) ok THREW Cannot read properties of undefined (reading 'delete')
8 base instanceof Request true true ok (both true — each against its own binding)
9 base.kind undefined user-sdk-request ✗ smoking gun

Cell 9 names the mechanism directly: base carries a field from a class the module never imported.

Node and tsc agree with bun — Request is simply not in scope from that import.

Root cause

crates/perry/src/commands/compile/run_pipeline.rs ~4790–4828, in the import handling:

// Mirror the namespace-import behavior: for every
// native-compiled module we import from (and every module that
// module transitively re-exports from), enumerate every class
// defined in that module and register it for dispatch, even
// when the class name wasn't in the specifier list. Local
// classes with the same name take precedence in
// `compile_module` (the `class_table.contains_key` check), so
// this doesn't clobber anything.
...
for class in &src_hir.classes {
    if !class.is_exported { continue; }
    if imported_classes.iter().any(|c| c.name == class.name) { continue; }
    ...
    imported_classes.push(imported_class_from_hir(...));
}

The over-registration is deliberate and its stated safety argument is that local classes of the same name win. That argument holds for local classes — but a global intrinsic is not a local class, so nothing outranks the implicit entry. Every name in is_builtin_global_value_name (crates/perry-hir/src/analysis/builtins.rs) is exposed: Request, Response, Headers, URL, Event, Blob, File, FormData, WebSocket, Error, Map, Set, …

A generated SDK exporting a class named Request/Response/Headers is extremely common (hey-api, openapi-typescript, oazapfts all do it), so this is not an exotic collision.

The entry lands in imported_class_ctors, which lower_call/new.rs consults before the intrinsic path — the disassembly of the real OpenCode binary confirms rewrite contains no js_request_new call at all, only js_new_target_set (the generic construct path) and a call to opencode_packages_sdk_js_src_v2_gen_sdk_gen_ts__Request_constructor.

Proposed fix

In that implicit registration loop only, skip classes whose name is a builtin global value name. Explicitly imported classes are pushed by the earlier specifier-driven sites and already win the any(|c| c.name == ...) dedup, so an explicit import { Request } from "./mod.js" keeps working — only the un-imported leak is removed.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions