Bug: identifiers in binding positions are quoted instead of sanitized, producing invalid or silently-wrong TypeScript #156

Summary

get_typescript_ident quotes any name containing a character outside [A-Za-z0-9_]:

https://github.com/dfinity/icp-js-bindgen/blob/main/src/core/generate/rs/src/bindings/typescript_native/utils.rs#L326-L343

Quoting is correct for property positions (record field labels, method names — 'my-field': bigint is legal). It is invalid for binding positions — declaration names and type references — where 'my-backend' is a syntax error, not an identifier. One function serves both kinds of caller, and the binding-position callers are unguarded.

That single confusion produces five distinct failures in the generated actor/wrapper file (<service-name>.ts) and the interface file (<service-name>.d.ts). Two of them are reachable with a perfectly ordinary .did filename, and one of those is silent.

All five verified against @icp-sdk/bindgen@0.4.0 (npm latest) and current main. The declarations/*.did.d.ts and declarations/*.did.js files are unaffected — they never receive the service name.

D — silent, ordinary filename (highest severity)

A .did whose basename capitalizes into a JavaScript built-in. The class declaration is keyword-guarded, the references are not:

printf 'service : () -> { test: () -> () }\n' > map.did
npx @icp-sdk/bindgen@0.4.0 --did-file ./map.did --out-dir ./out --force

export class Map_ implements mapInterface {                                          // guarded
export function createActor(canisterId: string, options: CreateActorOptions = {}): Map {  // NOT guarded
    return new Map(actor);                                                           // NOT guarded

createActor constructs a real JavaScript Map, so every service method is missing at runtime. There is no syntax error, and the file's // @ts-nocheck hides the type error. Stripping the header shows what a consumer never sees:

map.ts(68,84): error TS2314: Generic type 'Map<K, V>' requires 2 type argument(s).

Cause — declaration guarded at compile_wrapper.rs#L297, references unguarded at #L618 and #L866. Same class of name: set, record, date, error, promise, array, function, math.

E — parse error, ordinary filename

The inline all-null-variant enum name is built from the candid tags and then quoted:

printf 'service : () -> { f : () -> (variant { "my-tag"; other }) query }\n' > vtag.did

export enum 'Variant_other_my-tag' {                                    // invalid
    f(): Promise<'Variant_other_my-tag'>;                               // invalid
    return "other" in value ? Variant_other_my-tag.other : ...          // invalid

Cause: new_typescript_native_types.rs#L511-L519. Same code touched by #151/#152. Needs no unusual filename.

A — invalid identifiers from the .did basename

printf 'service : () -> { test: () -> () }\n' > my-backend.did
npx @icp-sdk/bindgen@0.4.0 --did-file ./my-backend.did --out-dir ./out --force
npx tsc --noEmit --skipLibCheck --target es2022 --module esnext --moduleResolution bundler ./out/my-backend.ts

export interface 'my-backendInterface' {                                                  // invalid
export class 'My-backend' implements 'my-backendInterface' {                              // invalid
export function createActor(canisterId: string, options: CreateActorOptions = {}): My-backend { // invalid

my-backend.ts(63,18): error TS2427: Interface name cannot be 'my-backendInterface'.
my-backend.ts(63,1):  error TS1128: Declaration or statement expected.
... 19 errors total

These are TS1xxx/TS2427 parse errors — the file's // @ts-nocheck does not suppress them, it only suppresses type errors. The service name reaches a binding position via service_interface_ident → get_ident_guarded (new_typescript_native_types.rs#L1113).

B — import specifier does not match the file written

The specifier rewrites - to _; the declarations file keeps the dash:

out/declarations/my-backend.did.d.ts     ← written
out/declarations/my-backend.did.js       ← written
out/my-backend.ts → imports "./declarations/my_backend.did"   ← does not exist

Cause: service_name.replace('-', "_") at imports.rs#L17, #L42, original_typescript_types.rs#L408, while the JS layer writes files from the unmodified basename (index.ts#L132, #L201-L206). Dash is the only mangled character — my.backend.did already round-trips correctly today.

C — panic on a multi-byte leading character

cp my-backend.did "ünicode.did"
npx @icp-sdk/bindgen@0.4.0 --did-file "./ünicode.did" --out-dir ./out --force
# [icp-bindgen] Error: unreachable

The class name is built as uppercase(first char) + &service_name[1..] — a byte slice, which splits a UTF-8 char boundary and panics. Duplicated in three places: compile_wrapper.rs#L142-L146, #L190-L194, #L549-L553. An ASCII first char with later multi-byte chars (aünicode.did) does not panic but still emits quoted identifiers.

Expected behavior

<service-name>.ts and <service-name>.d.ts should always be valid, compilable TypeScript, the import specifier should always resolve to a file that was written, and generation should never panic.

Scope of affected basenames

Workaround: restrict the .did basename to [A-Za-z0-9_] and avoid names that capitalize into a JS built-in; or pass --actor-disabled to emit only the (valid) declarations.

Suggested fix

One change, because all five share one cause: split get_typescript_ident into a property-name path (quote — unchanged) and a binding-identifier path (sanitize), then route every binding position through the latter.

The binding sanitizer must be identity on already-legal identifiers. This is load-bearing for compatibility: the interface name is built from the raw basename today (hello_worldInterface, not Hello_worldInterface — see tests/snapshots/generate/hello_world/hello_world.ts.snapshot) and is a documented contract (docs/src/content/docs/structure.md). Identity-on-valid keeps every existing output byte-identical, keeps this a patch-level change, and lets one helper safely serve both service names and candid type names — the Candid grammar restricts type ids to (letter|'_')(letter|digit|'_')*, so sanitizing is a no-op there by construction.

Rule: collapse each run of illegal characters to _; prefix _ if the leading character is not a legal ID_Start; keyword-guard using the existing KEYWORDS list rather than a second one. Use unicode-ident for the legality test (already present in Cargo.lock at 1.0.19) so ünicode.did keeps its name instead of degrading. my-backend → my_backend, which gives the useful property that renaming my-backend.did → my_backend.did produces identical output.

Then derive the class name once from that base, char-boundary-safely, and reuse the single value for the declaration, the createActor return type and the new X() callee — so they cannot drift (D) and the byte slicing disappears (C). Apply the same helper to the Variant_* enum name (E), and delete the three replace('-', "_") calls (B); once removed, the Rust specifier and the JS-written filename are the same string by construction.

Not in scope (separate issues)

• Moving the flattenImportPath string rewrite (index.ts#L234-L239) into Rust. Its "must stay in sync" comment marks real duplication, but it is a whole-file text replace and a different concern.
• A candid type whose name equals the generated class name collides today, independent of any of the above: my_backend.did containing type My_backend emits both export interface My_backend and export class My_backend. Needs a design decision (uniquifier vs. documented limitation).
• docs/src/content/docs/structure.md documents createActor as returning <service-name>Interface; it returns the class.

Environment

• @icp-sdk/bindgen@0.4.0 (npm latest) and current main; verified with TypeScript 5.9.3.