Skip to content

fix(types): accept Record types in query and execute values#4276

Open
uPaymeiFixit wants to merge 1 commit into
sidorares:masterfrom
uPaymeiFixit:fix/execute-values-object-branch
Open

fix(types): accept Record types in query and execute values#4276
uPaymeiFixit wants to merge 1 commit into
sidorares:masterfrom
uPaymeiFixit:fix/execute-values-object-branch

Conversation

@uPaymeiFixit

Copy link
Copy Markdown
Contributor

Closes #4275

  • Loosen the object branch on both types to { [key: string]: unknown }. Top-level and array-branch strictness stay intact, so undefined inside an execute values array is still rejected at compile time.
  • Adds a type-level regression test at test/tsc-build/strict-checks/values.test.ts covering top-level and nested Record<string, unknown> for both callback and promise APIs.
  • Updates the QueryValues docs section to cover ExecuteValues, fix stale type signatures from before fix(types): improve ExecuteValues "nested" params #4133, and document the undefined behavior difference between .query() and .execute().

@codecov

This comment was marked as off-topic.

@wellwelwel

wellwelwel commented Apr 24, 2026

Copy link
Copy Markdown
Collaborator

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 unknown into defined types (QueryValues, ExecuteValues). In this case, it would be more viable to go back to any and accept any value from the users and let them discover invalid types at runtime (which is what this PR is doing when a user passes objects).

Do you think we should revert both to any? The way I see it, a user who wants to deal with unknown without handling it and send it directly to the database should do something like:

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]);

@uPaymeiFixit

Copy link
Copy Markdown
Contributor Author

Sure, let me share my perspective on both directions: any vs. disallowing Record types.

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 any

My big motivation for creating the types in the first place, #3982, was because our organization was seeing runtime errors in production; not during testing. Primarily due to passing undefined values sometimes even though undefined is not a supported type. Both Claude Code and our own developers were passing in complicated types which contained possibly undefined values that really should have been filtered out or replaced with null.

any is just saying "don't use TypeScript here", so you lose any benefit of TypeScript for that variable. As MySQL2's types get better developers will start blindly trusting them more and more and so I predict an incorrect type such as any will actually start causing more issues.

Keeping { [key: string]: QueryValues }

My main issue with this is that it feels too restrictive. Both my PR and the existing type are incorrect. In the existing case Record types are allowed, but the type says they aren't. This forces the user, who may not be as familiar with MySQL2, to know that the type is incorrect and to use a type assertion (like your example). My personal experience with this was noticing that upgrading MySQL2 broke a lot of our types, and when Claude Code was fixing them it tried to stringify all of our objects. It took me far too long to remember that MySQL2 actually did support sending objects and to look into what I later discovered to be the new more restrictive ExecuteValues type.
tl;dr - it breaks documentation, new developers will assume MySQL doesn't support sending objects

What this PR's type does and doesn't do

I want to be clear, all three options (any, the existing type, and this PR) are all incorrect types. They all lie to the user about which types will result in a smooth runtime. I'm just trying to find the one that fixes the most common use cases.

Both the existing type and this PR prevent passing in direct undefined values; that's the biggest win IMO as it prevents runtime errors that we were actually experiencing in practice.

My PR allows through some types that likely aren't intended, but also don't throw and likely don't even have any significant data impact. For example, passing in [{ user: { name: 'alice', middleName: undefined } }] would now be allowed per the type, but at runtime middleName wouldn't show up in the database; that's probably fine. I can't find a case where this PR introduces something that hurts the user's experience either by throwing at runtime or significantly changing the data.

If it helps, here's a table of some use cases / edge cases and how they're handled by the current type (OLD) vs. this PR's type (NEW) vs. the runtime.

I won't die on this hill, but in my opinion every case I could find where the current and new PR's type don't match, this PR's type is what I would prefer given the value.


  • OLD / NEW — ✅ tsc accepts · ❌ tsc rejects
  • Runtime — what the mysql2 client sends to MySQL for this input. ✅ matches caller intent · ⚠️ silently drops or coerces · ❌ throws before the socket
Call OLD NEW Runtime
connection.execute(
'INSERT INTO data (json_col) VALUES (?)',
[{ user: { name: 'alice', onClick: () => 42 } }]
)
⚠️ JSON {"user":{"name":"alice"}}onClick omitted
connection.execute(
'INSERT INTO data (json_col) VALUES (?)',
[{ user: { name: 'alice', middleName: undefined } }]
)
⚠️ JSON {"user":{"name":"alice"}}middleName omitted
connection.execute(
'INSERT INTO data (json_col) VALUES (?)',
[{ user: { id: Symbol('s') } }]
)
⚠️ JSON {"user":{}}id omitted
connection.execute(
'INSERT INTO data (json_col) VALUES (?)',
[{ handlers: [() => 42, () => 42] }]
)
⚠️ JSON {"handlers":[null,null]} — functions replaced with null
connection.execute(
'INSERT INTO data (json_col) VALUES (?)',
[{ values: [undefined, 1, 2] }]
)
⚠️ JSON {"values":[null,1,2]}undefined replaced with null
connection.execute(
'INSERT INTO data (json_col) VALUES (?)',
[{ user: { get onClick() { return () => 42 } } }]
)
⚠️ JSON {"user":{}} — getter result omitted
connection.execute(
'INSERT INTO data (json_col) VALUES (?)',
[{ name: 'alice', middleName: undefined }]
)
⚠️ JSON {"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 = o
connection.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)

@uPaymeiFixit

Copy link
Copy Markdown
Contributor Author

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 { [key: string]: unknown }.

For example, a valid concern from the second item in my table above could be:

I'm concerned a user will write

const values = [{ user: { name: 'alice', middleName: undefined }}];
const sql = 'INSERT INTO data (json_col) VALUES (?)';
connection.execute(sql, values)

And then be surprised middleName was omitted instead of NULL, or allowed by the type at all.

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).

@wellwelwel

wellwelwel commented May 23, 2026

Copy link
Copy Markdown
Collaborator

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]);

My personal point is precisely that this shouldn't be MySQL2's role. For example, as I said in the previous comment:

In my view, it shouldn't be MySQL2's role to identify and perform narrowing of unknown into defined types (QueryValues, ExecuteValues). In this case, it would be more viable to go back to any and accept any value from the users and let them discover invalid types at runtime (which is what this PR is doing when a user passes objects).

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 unknown as a whole (not just for Record<string, unknown>), but MySQL2 doesn't wait for any input and then validates according to each typeof, for example. Instead, it can indeed break (intentionally) with incorrect parameters. At the user level, using unknown for this case would have the same effect as any, because the end user will be able to pass any parameter and none will throw an error, besides implying that there is handling for all types internally, which doesn't actually happen.

Regardless of unknown, is there a specific type that should be accepted but isn't covered by the current types, or the opposite, accepting some type that shouldn't be accepted?

@wellwelwel

wellwelwel commented May 24, 2026

Copy link
Copy Markdown
Collaborator

Justifying why I say this shouldn't be MySQL2's responsibility:

This could set a precedent like "if it's a parameter, it's unknown" throughout the entire codebase, since any user could "mask" a type behind unknown (like in the example I showed with const values: unknown = 'Hey U 🌞'). This wouldn't only affect MySQL2, but practically all TypeScript projects where a user can define a parameter and force unknown.

On the side of the user who actually wants to use unknown but doesn't want to handle it before sending it as a parameter (speaking at the API usage level), they should take an approach like as ExecuteValues or as QueryValues, not because of MySQL2, but because of TypeScript itself. If the user really wants to send any value and "override" the valid types, then they can use as any (it's not a recommendation 😅). For example, backing to Record:

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 unknown as a parameter that is actually handled internally in another project I develop vs. when the type is unknown but the responsibility is kept with the user 🙋🏻‍♂️

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression: ExecuteValues no longer allows Record type

3 participants