kysely-org / kysely

A type-safe typescript SQL query builder
https://kysely.dev
MIT License
10.71k stars 271 forks source link

Bigint prevents use on edge environments #142

Closed t3dotgg closed 2 years ago

t3dotgg commented 2 years ago

Tried using kysely in an Astro edge env as a string builder for the new planetscale http client. Was able to work around with a nasty patch package lol. Literally just had to change the lock id for postgres to not be a bigint

image

Patch:

diff --git a/node_modules/kysely/dist/cjs/dialect/postgres/postgres-adapter.js b/node_modules/kysely/dist/cjs/dialect/postgres/postgres-adapter.js
index 08ef143..301315f 100644
--- a/node_modules/kysely/dist/cjs/dialect/postgres/postgres-adapter.js
+++ b/node_modules/kysely/dist/cjs/dialect/postgres/postgres-adapter.js
@@ -4,7 +4,7 @@ exports.PostgresAdapter = void 0;
 const sql_js_1 = require("../../raw-builder/sql.js");
 const dialect_adapter_base_js_1 = require("../dialect-adapter-base.js");
 // Random id for our transaction lock.
-const LOCK_ID = 3853314791062309107n;
+const LOCK_ID = 3853314791062309107;
 class PostgresAdapter extends dialect_adapter_base_js_1.DialectAdapterBase {
     get supportsTransactionalDdl() {
         return true;
diff --git a/node_modules/kysely/dist/esm/dialect/postgres/postgres-adapter.js b/node_modules/kysely/dist/esm/dialect/postgres/postgres-adapter.js
index 2596e72..c48e562 100644
--- a/node_modules/kysely/dist/esm/dialect/postgres/postgres-adapter.js
+++ b/node_modules/kysely/dist/esm/dialect/postgres/postgres-adapter.js
@@ -2,7 +2,7 @@
 import { sql } from '../../raw-builder/sql.js';
 import { DialectAdapterBase } from '../dialect-adapter-base.js';
 // Random id for our transaction lock.
-const LOCK_ID = 3853314791062309107n;
+const LOCK_ID = 3853314791062309107;
 export class PostgresAdapter extends DialectAdapterBase {
     get supportsTransactionalDdl() {
         return true;
t3dotgg commented 2 years ago

Github repo w/ this working btw https://github.com/TheoBr/pscalebench

koskimas commented 2 years ago

I think that number gets truncated, as it's greater than the largest integer that can be expressed as a JavaScript number. Does a string work?

t3dotgg commented 2 years ago

@koskimas Yes string works as well (at least passes compilation - I'm not using the driver)

Not sure about truncation, will bring up w/ the Astro guys as the warning seems to exist in that layer

koskimas commented 2 years ago

Kysely has a minimum supported node version (14) that does support bigints, but I can fix this particular issue if you can come up with a patch that works.

Kysely can break for those old node and browser versions at any given time. There is no guarantee that things will work with node version below 14 (or any browser that has a subset of node 14 features implemented).

The minimum version is in the package.json file

{
  "name": "kysely",
  "version": "0.21.3",
  "description": "Type safe SQL query builder",
  "repository": {
    "type": "git",
    "url": "git://github.com/koskimas/kysely.git"
  },
  "engines": {
    "node": ">=14.0.0"
  },
...
t3dotgg commented 2 years ago

To be clear this isn't an old node version - this is edge support (similar to Deno if anything)

koskimas commented 2 years ago

Kysely only officially supports node. You need to compare the features to the minimum supported node version, which is 14.

The edge environment definitely supports bigints. I think almost any deno version does. It's just a problem with your bundler settings.

jackharrhy commented 2 years ago

Running into this issue while trying to use Astro, not even using an edge environment here, seems to break with Astro's dev server (npm run dev)

igalklebanov commented 2 years ago

Had similar issues with n notation and applied a similar patch a few months ago - node 14.x, but esbuild configured with a target that doesn't support it.

The following is truthy in chrome, node 16.x, node 14.x, deno 1.26.x:

BigInt('3853314791062309107') === 3853314791062309107n

And most importantly, astro dev starts successfully after patching this in.

igalklebanov commented 1 year ago

@t3dotgg Fix released in Kysely 0.23.