Skip to content

Cross-module tagged template literal: rest params not packed into array at callee #608

Description

@proggeramlug

Repro

// node_modules/repro-pkg2/sql.js
export function sql(strings, ...params) {
  console.log('strings.length:', strings && strings.length);
  console.log('params:', params);
  console.log('params.length:', params && params.length);
}
// probe_tagged_xmod.ts (with repro-pkg2 in package.json#perry.compilePackages)
import { sql } from 'repro-pkg2';

sql`default`;             // 0 interp
const x = "world";
sql`hello ${x}`;          // 1 interp

Bun output (correct):

strings.length: 1
params: []
params.length: 0
strings.length: 2
params: [ "world" ]
params.length: 1

Perry output (bug):

strings.length: 1
params: undefined
params.length: undefined        ← rest param not bound at all
strings.length: 2
params: world                   ← rest param is the raw arg, not [arg]
params.length: 5                ← reads the string's .length, not array length

The strings array is fine in both cases (perry passes the static-string array correctly). But the rest parameter ...params is broken:

  • 0 interpolations → params === undefined (should be [])
  • 1 interpolation → params === firstArg (should be [firstArg])

Looks like the tagged-template call site emits sql(strings, ...interp_args) as plain positional args without packing the trailing args into a rest array; the callee's ...params rest binding then either doesn't fire (no trailing args) or gets the single arg directly instead of a 1-element array.

What works

Same-module tagged template literal works correctly:

// probe_tagged.ts — sql defined and used in the same file
function sql(strings: any, ...params: any[]) { ... }
sql`default`;     // params = []  ✅
sql`hello ${x}`;  // params = [x] ✅

So the bug is specifically the cross-module / compilePackages call path — whatever shape lowers sql from an imported ExternFuncRef doesn't honour the callee's rest-param signature.

Why it matters for #488

Blocks the drizzle + @perryts/postgres acceptance test, immediately after #597 unblocked the for-of-on-Any-typed-arr.entries() path.

drizzle's dialect.buildInsertQuery calls valueList.push(sql\default`)(drizzle-ormpg-core/dialect.js:390), where sqlis the tagged-template helper imported from'../sql/sql.js'`. Inside that helper:

function sql(strings, ...params) {
  const queryChunks = [];
  if (params.length > 0 || strings.length > 0 && strings[0] !== "") { ... }
  for (const [paramIndex, param2] of params.entries()) { ... }
  ...
}

params.length reads undefined.length → throws TypeError: Cannot read properties of undefined (reading 'length'). drizzle uses sql\...`` everywhere — buildInsertQuery / buildSelection / buildJoins / buildSetOperations / buildSelectQuery — so almost every code path that builds SQL hits this.

Where I'd start digging

The closure-arity registry from #493 (codegen-emitted call site bundles trailing args into a rest array when the callee has a rest-last param) likely doesn't fire for tagged-template-literal call sites in cross-module dispatch. Two surfaces:

  1. HIR lowering of tagged-template (crates/perry-hir/src/lower/...) — when the tag function is an imported ExternFuncRef, the lowered call should carry the same has_rest flag that ordinary cross-module calls do. Check whether the tagged-template lowering goes through the same imported_func_param_counts / rest-aware dispatch path as a regular f(...args) call.

  2. Codegen call site (crates/perry-codegen/src/lower_call.rs) — if HIR is emitting a normal Call with the right args, codegen needs to consult imported_func_has_rest (or whatever the equivalent map is) and emit js_array_alloc(n) + populate, then pass that array as the trailing arg.

Same-module case works because the HIR has the local function's full signature in func_signatures and bundles correctly.

Workaround

Patch the user-side helper to detect undefined params and synthesize an empty array, plus handle the single-arg case. Cleaner is to fix on perry's side.

Refs

Blocks #488. After this lands, the next failure surface is whatever fires once sql\default`` returns a real SQL object — haven't reached that frame yet.

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