fix(types): accept Record types in query and execute values#4276
fix(types): accept Record types in query and execute values#4276uPaymeiFixit wants to merge 1 commit into
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Thanks, @uPaymeiFixit! I saw the #4275 and I can't see this as a regression. In my view, it shouldn't be MySQL2's role to identify and perform narrowing of Do you think we should revert both to const value: Record<string, unknown> = { hello: 'world' };
query('INSERT INTO data (json_col) VALUES (?)', [value as QueryValues]); // <-Full example: export type QueryValues = /* or ExecuteValues */
| string
| number
| bigint
| boolean
| Date
| null
| Blob
| Buffer
| Uint8Array
| QueryValues[]
| { [key: string]: QueryValues };
declare function query(sql: string, values?: QueryValues): void;
const value: Record<string, unknown> = { hello: 'world' };
query('INSERT INTO data (json_col) VALUES (?)', [value as QueryValues]); |
|
Sure, let me share my perspective on both directions: But let me start with this. I think TypeScript has two huge benefits / goals: preventing runtime errors, and providing documentation. I think both of those directions breaks one or both of those goals. Reverting to
|
| Call | OLD | NEW | Runtime |
|---|---|---|---|
connection.execute( 'INSERT INTO data (json_col) VALUES (?)', [{ user: { name: 'alice', onClick: () => 42 } }]) |
❌ | ✅ | {"user":{"name":"alice"}} — onClick omitted |
connection.execute( 'INSERT INTO data (json_col) VALUES (?)', [{ user: { name: 'alice', middleName: undefined } }]) |
❌ | ✅ | {"user":{"name":"alice"}} — middleName omitted |
connection.execute( 'INSERT INTO data (json_col) VALUES (?)', [{ user: { id: Symbol('s') } }]) |
❌ | ✅ | {"user":{}} — id omitted |
connection.execute( 'INSERT INTO data (json_col) VALUES (?)', [{ handlers: [() => 42, () => 42] }]) |
❌ | ✅ | {"handlers":[null,null]} — functions replaced with null |
connection.execute( 'INSERT INTO data (json_col) VALUES (?)', [{ values: [undefined, 1, 2] }]) |
❌ | ✅ | {"values":[null,1,2]} — undefined replaced with null |
connection.execute( 'INSERT INTO data (json_col) VALUES (?)', [{ user: { get onClick() { return () => 42 } } }]) |
❌ | ✅ | {"user":{}} — getter result omitted |
connection.execute( 'INSERT INTO data (json_col) VALUES (?)', [{ name: 'alice', middleName: undefined }]) |
❌ | ✅ | {"name":"alice"} — middleName omitted |
const r: Record<string, unknown> = { id: 1, name: 'alice' }connection.execute( 'SELECT :id, :name', r) |
❌ | ✅ | ✅ ?=1, ?='alice' (positional after named-placeholder rewrite) |
const r: Record<string, unknown> = { hello: 'world' }connection.execute( 'INSERT INTO data (json_col) VALUES (?)', [r]) |
❌ | ✅ | ✅ JSON {"hello":"world"} |
class User { constructor(public name: string) {} }connection.execute( 'INSERT INTO data (json_col) VALUES (?)', [{ user: new User('alice') }]) |
❌ | ✅ | ✅ JSON {"user":{"name":"alice"}} |
class User { constructor(public name: string) {} toJSON() { return { serialized_name: this.name } }}connection.execute( 'INSERT INTO data (json_col) VALUES (?)', [{ user: new User('alice') }]) |
❌ | ✅ | ✅ JSON {"user":{"serialized_name":"alice"}} |
class User { constructor(public name: string) {} toJSON() { return { serialized_name: this.name } }}connection.execute( 'SELECT :id', { id: new User('alice') }) |
❌ | ✅ | ✅ JSON {"serialized_name":"alice"} |
connection.execute( 'SELECT ?', [Symbol('x')]) |
❌ | ❌ | ?='Symbol(x)' — coerced via toString() |
const r: Record<string, unknown> = { hello: 'world' }connection.query( 'INSERT INTO data (json_col) VALUES (?)', [r]) |
✅ | ✅ | INSERT INTO data (json_col) VALUES ('[object Object]') |
connection.query( 'INSERT INTO data (json_col) VALUES (?)', [{ user: { name: 'alice', onClick: () => 42 } }]) |
✅ | ✅ | INSERT INTO data (json_col) VALUES ('[object Object]') |
connection.query( 'INSERT INTO data (json_col) VALUES (?)', [{ user: { name: 'alice', middleName: undefined } }]) |
✅ | ✅ | INSERT INTO data (json_col) VALUES ('[object Object]') |
connection.query( 'SELECT ?', [() => 42]) |
✅ | ✅ | SELECT '() => 42' — function source inlined as SQL literal |
connection.execute( 'SELECT ?', [new Uint8Array([1, 2, 3])]) |
✅ | ✅ | ?='1,2,3' — coerced via toString(), not sent as binary |
const o: { self?: typeof o } = {}o.self = oconnection.execute( 'INSERT INTO data (json_col) VALUES (?)', [o]) |
✅ | ✅ | ❌ throws TypeError: Converting circular structure to JSON |
connection.execute( 'INSERT INTO data (json_col) VALUES (?)', [{ count: 123n }]) |
✅ | ✅ | ❌ throws TypeError: Do not know how to serialize a BigInt |
const p = new Proxy({}, { get() { throw new Error('trap') } })connection.execute( 'INSERT INTO data (json_col) VALUES (?)', [p]) |
✅ | ✅ | ❌ throws Error: trap (user code throws inside serialization) |
connection.execute( 'SELECT ?, ?, ?', [1, 'x', null]) |
✅ | ✅ | ✅ ?=1, ?='x', ?=NULL |
connection.execute( 'SELECT :id, :name', { id: 1, name: 'alice' }) |
✅ | ✅ | ✅ ?=1, ?='alice' |
connection.execute( 'SELECT ?, ?', [new Date('2026-01-01T00:00:00Z'), Buffer.from('hello')]) |
✅ | ✅ | ✅ ?=<DATETIME>, ?=<BLOB> |
connection.execute( 'INSERT INTO data (json_col) VALUES (?)', [{ hello: 'world', count: 3 }]) |
✅ | ✅ | ✅ JSON {"hello":"world","count":3} |
const u: { name: string; middleName?: string } = { name: 'alice' }connection.execute( 'INSERT INTO data (json_col) VALUES (?)', [u]) |
✅ | ✅ | ✅ JSON {"name":"alice"} |
connection.execute( 'SELECT ?', [123n]) |
✅ | ✅ | ✅ ?='123' — coerced via toString() |
connection.query( 'SELECT ?', [undefined]) |
✅ | ✅ | ✅ SELECT NULL |
connection.execute( 'SELECT ?', [undefined]) |
❌ | ❌ | ❌ throws TypeError: Bind parameters must not contain undefined. To pass SQL NULL specify JS null |
connection.execute( 'SELECT ?', [() => 42]) |
❌ | ❌ | ❌ throws TypeError: Bind parameters must not contain function(s). |
connection.execute( 'SELECT ?', [new Map([['a', 1]])]) |
❌ | ❌ | ❌ throws TypeError from StringParser.encode (unsupported argument type) |
connection.execute( 'SELECT ?', [new Set([1, 2, 3])]) |
❌ | ❌ | ❌ throws TypeError from StringParser.encode (unsupported argument type) |
|
I think a useful way to discuss this could be to talk about what challenges we're afraid users will have if we change the types to allow For example, a valid concern from the second item in my table above could be:
Or on the opposite side of the argument, my legitimate concern if we don't make this change is that someone will write code like: const values: Record<string, unknown> = { hello: 'world' }
const sql = 'INSERT INTO data (json_col) VALUES (?)';
connection.execute(sql, [values]);And the current type won't allow them, so they'll have to loosen their types via type assertions (what my organization is currently having to do) which could lead to bugs in the future if the shape ever changes (and now the assertion asserts the incorrect type). |
My personal point is precisely that this shouldn't be MySQL2's role. For example, as I said in the previous comment:
I see this exactly the same way as using: const values: unknown = 'Hey U 🌞';
const sql = 'INSERT INTO data (msg) VALUES (?)';
connection.execute(sql, [values]);Thinking this way, we would need to define Regardless of |
|
Justifying why I say this shouldn't be MySQL2's responsibility: This could set a precedent like "if it's a parameter, it's On the side of the user who actually wants to use type A = Record<string, ExecuteValues>;
type B = Record<string, QueryValues>;
type C = Record<string, string | number | "etc.">;
type D = Record<string, any>;@uPaymeiFixit, if you want, I can show some real examples of |
Closes #4275
{ [key: string]: unknown }. Top-level and array-branch strictness stay intact, soundefinedinside anexecutevalues array is still rejected at compile time.test/tsc-build/strict-checks/values.test.tscovering top-level and nestedRecord<string, unknown>for both callback and promise APIs.QueryValuesdocs section to coverExecuteValues, fix stale type signatures from before fix(types): improveExecuteValues"nested" params #4133, and document theundefinedbehavior difference between.query()and.execute().