search

planetscale / database-js

A Fetch API-compatible PlanetScale database driver
https://planetscale.com/docs/tutorials/planetscale-serverless-driver
Apache License 2.0
1.17k stars 35 forks source link

coerce rowsAffected and insertId to concrete values #89

Closed mattrobenolt closed 1 year ago

mattrobenolt commented 1 year ago

The fact that they come over the wire as null is a side effect of the protojson encoding, when these fields are uint64 and can't actually be null. An empty value should be treated as an equivalent "empty" value when inflating.

The side effect of this is making rowsAffected and lastId not nullable anymore on ExecutedQuery.

The slightly weird part about this is both rowsAffected and lastId are uint64 (a bigint), but rowsAffected is being lossily coerced into a JavaScript number, while lastId is left as a string.

I don't know if we should address being able to coerce things into a BigInt when it's applicable or not. I don't know how JavaScript ecosystem works.

Fixes #88

mattrobenolt commented 1 year ago

In what cases can we not just use BigInt? Seems pretty ubiquitous? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt

I wouldn't want to do this in this PR, but I think a larger version bump and if a BigInt is available, it seems we should just generically use this for all u?int64s automatically.

mattrobenolt commented 1 year ago

I've opened up #90 with a stab at just simply us using bigint types where applicable.