RobinBlomberg / kysely-codegen

Generate Kysely type definitions from your database.
MIT License
807 stars 73 forks source link

Why is `kysely-codegen` not able to connect to the PostgreSQL database with the connection string, when `kyself` itself is able to? #65

Closed magnusrodseth closed 1 year ago

magnusrodseth commented 1 year ago

I have a serverless TimescaleDB database running that I'm trying to autogenerate types from. Using the following config in my lib/kysely.ts, I successfully connect to the database:

lib/kysely.ts

import SensorData from "../src/schemas/public/SensorData";
import { Pool } from "pg";
import { Kysely, PostgresDialect } from "kysely";

type Database = {
  sensor: SensorData;
};

export const db = new Kysely<Database>({
  dialect: new PostgresDialect({
    pool: new Pool({
      database: process.env.TIMESERIES_DB_NAME,
      port: Number(process.env.TIMESERIES_DB_PORT),
      user: process.env.TIMESERIES_DB_USERNAME,
      host: process.env.TIMESERIES_DB_HOST,
      password: process.env.TIMESERIES_DB_PASSWORD,
    }),
  }),
});

However, this SensorData is basically manually typed, and I want to use kysely-codegen to do this for me.

I use the following script, as I already have a MySQL database with Prisma pointing to DATABASE_URL:

  "scripts": {
    "with-env": "dotenv -e ../../.env --",
    "gen:types": "pnpm with-env kysely-codegen --url 'env(TIMESERIES_DB_CONNECTION_STRING)'"
  },

My environment variables are successfully loaded, as I get the following output in my terminal:

> dotenv -e ../../.env -- "kysely-codegen" "--url" "env(TIMESERIES_DB_CONNECTION_STRING)"

• Loaded environment variables from .env file.
/Users/magnusrodseth/dev/jrc/effisense/sensor-dashboard/node_modules/.pnpm/kysely-codegen@0.9.0_kysely@0.23.5+pg@8.10.0/node_modules/kysely-codegen/dist/connection-string-parser.js:57
                throw new SyntaxError(`Invalid URL: '${connectionString}'`);
                      ^

SyntaxError: Invalid URL: 'postgres://<username>:<password>@<host>:<port>/<name>?sslmode=require'
    at ConnectionStringParser.parse (/Users/magnusrodseth/dev/jrc/effisense/sensor-dashboard/node_modules/.pnpm/kysely-codegen@0.9.0_kysely@0.23.5+pg@8.10.0/node_modules/kysely-codegen/dist/connection-string-parser.js:57:23)

I think the problem that arises is the fact that by manually setting each property in lib/kysely.ts, I successfully connect to the database, but when passing the whole connection string TIMESERIES_DB_CONNECTION_STRING, this fails. My suspicion is further strengthened because the database connection previously failed when I tried to only use the connection string in lib/kysely.ts.

Why is kysely-codegen not able to connect to the PostgreSQL database with the connection string, when kyself itself is able to using an object as described above? How can we fix this?

magnusrodseth commented 1 year ago

I assume the code throws here in connection-string-parser.ts:

if (inferredDialectName !== 'sqlite') {
      try {
        void new URL(connectionString);
      } catch {
        throw new SyntaxError(`Invalid URL: '${connectionString}'`);
      }
    }
RobinBlomberg commented 1 year ago

Good find. Perhaps void new URL(connectionString); isn't a good enough check to see whether the connection string is valid. I'll look into it. Perhaps that check should be removed entirely, and we'll just let the connection fail if it's invalid.

magnusrodseth commented 1 year ago

@RobinBlomberg Would you be willing to remove it, so I could test it out ASAP? Alternatively, I could submit a PR if it would be just removing the lines highlighted above.

Subwaytime commented 1 year ago

Getting the same error on using mysql, i assume that the env parsing doesnt properly work with certain passwords or usernames. It seems to work if there arent any special characters in the .env.

magnusrodseth commented 1 year ago

@RobinBlomberg Will there be created a new release after 0.9.0 that includes this fix?

RobinBlomberg commented 1 year ago

@RobinBlomberg Will there be created a new release after 0.9.0 that includes this fix?

Of course! I will create a 0.9.1 soon (maybe today) as soon as I have the time.