dagger icon indicating copy to clipboard operation
dagger copied to clipboard

NodeJS SDK: query buider args as strings

Open slumbering opened this issue 3 years ago • 4 comments

To be more consistent with Go and Python SDKs, we should pass strings as arguments instead of object with the NodeJS query builder.

// BEFORE
const node = await client.container().from({ address: `node:${nodeVersion}` })
// AFTER
const node = await client.container().from(`node:${nodeVersion}`)

slumbering avatar Nov 14 '22 15:11 slumbering

Since TS doesn't have named parameters, the same can be done as in Go for the optionals. I.e., make required parameters positional, and put optionals in an opts argument typed in an interface.

const node = await client.container().from(`node:${nodeVersion}`).exec({args: ["node", "--version"]})

helderco avatar Nov 14 '22 15:11 helderco

We are actually rolling back to object as it fills more natural and we also avoid positional arguments.

slumbering avatar Nov 22 '22 17:11 slumbering

Between all positional or all in an object, I suggested the middle ground of positional required arguments, and an object for optionals (opts). What's wrong with that? Seems the better DX.

helderco avatar Nov 22 '22 17:11 helderco

TBD with @dolanor as it imply codegen work

slumbering avatar Nov 24 '22 09:11 slumbering