Node-less version

[steida]

I know it's not Kysely goal to be multi-platform aka universal, still, I believe it would be a very nice addition with relatively minimal effort. With the advent of local-first software, especially awesome SQLite, it would be nice to be able to use Kysely in browsers or React Native without hacks.

I'm opening an issue because this can be helpful for someone else. Maybe it should be mentioned in the readme.

webpack: (config) => {
  // Remove kysely server deps.
  config.resolve.fallback = {
    ...config.resolve.fallback,
    mysql2: false,
    pg: false,
    crypto: false,
    path: false,
    fs: false,
  };
  return config;
},

From the architectural point of view, I don't see a reason why a typed string builder should depend on Node.js at all. The single-responsibility principle FTW.

[koskimas]

From the architectural point of view, I don't see a reason why a typed string builder should depend on Node.js at all.

You are right. However, Kysely is not just a typed string builder.

[steida]

Not sure what has been changed (my code?), but this is enough now.

webpack: (config) => {
  // Remove kysely server deps.
  config.resolve.fallback = {
    ...config.resolve.fallback,
    path: false,
    fs: false,
  };

  return config;
},

[steida]

Suppress webpack warnings.

// https://github.com/koskimas/kysely/issues/33
if (process.env.NODE_ENV === 'development') {
  const warn = console.warn;
  console.warn = (...args) => {
    const isUselessWarning =
      typeof args[0] === 'string' &&
      args[0].includes(
        'Critical dependency: the request of a dependency is an expression',
      );
    if (!isUselessWarning) warn(...args);
  };
}

[koskimas]

I moved code like await import('pg') behind a helper importNamedFunction('pg', 'Pool') and the helper calls await import(moduleName). Instead of a string literal, import now takes a string variable (an expression). That's why you get the warning now too.

[barthuijgen]

I would love to get this working in the Deno runtime. I've tried this using their node compatibility mode and using esbuilder tools like https://www.skypack.dev/view/kysely but could not get it to work.

You are right. However, Kysely is not just a typed string builder.

I get this, but maybe to some that's all they need. I would like to use Kysely for string building and plug the strings into a database client that Deno supports.

Would be amazing to maybe split the package in two? one for typed string building, another for the rest?

[sampbarrow]

+1. I'd be interested in using this with sql.js in-browser. Would just need to abstract the sqlite driver (keeping the dialect stuff in place but allowing it to use sql.js instead of better-sqlite) and separate out the node dependent stuff.

[koskimas]

Splitting out the query building from the rest of the stuff would require a quite significant refactoring of the whole library. I'd rather make the library work in the browser (as it already does as steida has proven) and maybe in Deno too. We should be able to achieve that by dropping all possible node (and external) dependencies and importing the rest dynamically only when needed.

Currently there are node dependencies in migration.ts and postgres-dialect.ts files. The postgres-dialect dependencies are only for types. We could make migration.ts import the node stuff only when needed (migrations read from folder) but we can't easily get rid o the postgres typing dependencies.

One possibility would be to create a separate npm package for each dialect. You'd install kysely like this (on node):

npm install kysely kysely-postgres

but then we get to a new problem. You'd still want to use parts of the dialects (the query compiler and adapter) in the browser and deno. Where would we put those? A third package? I don't think so. In kysely core? Kind of feels wrong.

[sampbarrow]

One possibility would be to create a separate npm package for each dialect. You'd install kysely like this (on node):

This makes sense to me.

You'd still want to use parts of the dialects (the query compiler and adapter) in the browser and deno. Where would we put those? A third package? I don't think so. In kysely core? Kind of feels wrong.

From what I can tell, these are relatively small? I just made my own dialect that works with sql.js by copying the sqlite dialect and changing a few things in under an hour and am running it in the browser now (although I still get warnings about dynamic imports).

[koskimas]

From what I can tell, these are relatively small?

Currently yes. But if we add mssql or oracle or something, that don't follow the sql standards at all, we will have query compilers and adapters with a bunch of stuff you don't want to copy-paste.

[sampbarrow]

Well the most organized way would be a third package (although the dialect would be a dependency of the driver so the user would only need to manually install 2 packages).

On a related note I'd like to publish my absurd-sql kysely driver if you're interested in collaborating on it, I could use some input.

[koskimas]

I don't think Kysely will officially support a driver for absurd-sql. The project's README starts with "this is an absurd project". 😄

[steida]

@koskimas It's the future, no kidding. https://jlongster.com/future-sql-web

[koskimas]

Yep, I'm actually reading that right now and I'm impressed! The name should be changed 😄 I've always just ignored the whole thing because of the name. Sounds like an insane "just because I can" POC, but it isn't.

[sampbarrow]

Sounds like an insane "just because I can" POC, but it isn't.

Well it kind of is but it's still a cool library and has been mostly stable for me.

That being said, the driver would be more of a generic web worker driver. Since it has to run in a worker, the driver has to communicate with the worker. Then an adapter that runs inside the worker and exposes a couple of methods to the driver, but with another adapter you could use a similar library, the adapter only needs 3 or 4 methods. You could even do some kind of connection pooling with multiple workers, although I'm not sure how much real performance benefit that would bring.

This is what I'm doing and I have not really had any issues. I just had to move the connectionmutex behind the worker since I have two kysely instances accessing the same DB object, one from within the worker, one outside.

[steida]

@midwesterntechnology I implemented AbsurdSqlConnection in the worker and exposed it via comlink lib. Btw, why you are using two Kysely instances?

[sampbarrow]

@midwesterntechnology I implemented AbsurdSqlConnection in the worker and exposed it via comlink lib. Btw, why you are using two Kysely instances?

I'm using comlink too.

I have a couple of heavier background processes I had to move to the worker or they would lock up the main thread. They both use kysely as well.

[steida]

@midwesterntechnology Did you think about how to leverage db.prepare with Kysely? I am thinking about bulk imports performance.

[sampbarrow]

@midwesterntechnology Did you think about how to leverage db.prepare with Kysely? I am thinking about bulk imports performance.

Honestly I did not even try, just switched it to use exec for simplicity. I get a few hundred inserts per second when I'm in a transaction which is good enough for this app.

Edit: It seems kind of hacky, but I guess you could just prepare and then cache a prepared copy of each statement that your worker receives from kysely, then on subsequent executions if the string matches, pull up the cached statement and use that with the new params. You'd have to manage the cache though. You could clear the cache at the end of every transaction since bulk imports will always be within a transaction anyway, that would be relatively simple.

[steida]

@midwesterntechnology Can you share please how "few hundreds inserts" query looks like? Are huge SQL queries safe? Isn't there some limit? Thank you.

[sampbarrow]

@midwesterntechnology Can you share please how "few hundreds inserts" query looks like? Are huge SQL queries safe? Isn't there some limit? Thank you.

They are separate queries wrapped in one transaction, not one large query. That being said I have run 1000+ row bulk insert queries on kysely + absurd-sql and they worked fine from what I remember.

[barthuijgen]

It seems to be going a bit off topic, maybe open a new issue? Would be great to keep this related to the original issue.

[koskimas]

As the first step towards node-independence I'm refactoring the migration module to not depend on any node modules. The simplest way to do this seems to be providing an API like this. It's not the best, but simple enough for node users and should be extendable to any other file system implementation:

import { promises as fs } from 'fs'
import * as path from 'path'

const { error, results } = await db.migration.migrateToLatest(
  new FileMigrationProvider(fs.readdir, path.join, './migrations')
)

So you need to provide a function for reading files and joining file paths for a FileMigrationProvider. The FileMigrationProvider will be provided by Kysely, but the API for migration providers is trivial:

export interface MigrationProvider {
  getMigrations(): Promise<Record<string, Migration>>
}

After this, we only need to split out the dialects as separate packages and Kysely no longer depends on node or any other environment.

[sampbarrow]

Would it maybe make more sense to do something like:

const migrator = await db.migration(
  new FileMigrationProvider(fs.readdir, path.join, './migrations')
)
const { error, results } = await migrator.migrateToLatest();

This way we have a migration object that we can call different methods on (like migrateDown if/when that is added).

[koskimas]

That's a good idea. What if we move the migration provider all the way to the config? As an optional field of course, if the user doesn't need migrations.

const db = new Kysely<Database>({
  dialect: new PostgresDialect({
    host: 'localhost',
    database: 'kysely_test',
  }),

  migrationProvider: new FileMigrationProvider(
    fs.readdir, 
    path.join, 
    './migrations'
  )
})

What do you think? This way the db.migration module would remain just a "namespace" just like the db.schema and db.dynamic modules.

[sampbarrow]

Personally I'd leave it separate just because it's more loosely coupled that way, but I can't think of a situation in which it would be a problem so either way makes sense to me.

[koskimas]

If you want to make the migrations loosely coupled, then the best option would be something like this:

const migrator = new Migrator({
  db, // <-- This is a `Kysely` instance.
  provider: new FileMigrationProvider(
    fs.readdir, 
    path.join, 
    './migrations'
  )
})

And removing the db.migrate module alltogether

[sampbarrow]

If you want to make the migrations loosely coupled, then the best option would be something like this:

const migrator = new Migrator({
  db, // <-- This is a `Kysely` instance.
  provider: new FileMigrationProvider(
    fs.readdir, 
    path.join, 
    './migrations'
  )
})

And removing the db.migrate module alltogether

To me that is the ideal way to do it, but that's a larger change which is why I didn't suggest it.

[koskimas]

Let's go with the separate Migrator class anyway. It's the best solution. The reason for the coupling is easily solvable otherwise.

[sampbarrow]

Agreed.

[koskimas]

To make Kysely completely independent of any environment or packages, we'd need to create two packages per dialect as discussed earlier. That's already 6 packages, 6 documentation sites, 6 repositories for me to manage. That's too much work. Develpment will also be a PITA with two-way dependencies. Tools like lerna etc. cause more problems than they solve in my experience.

Also even though I changed the migration module to not require fs or path modules anymore, it still uses dynamic imports to import the migration files. Doesn't that still cause problems for you?

What are the current problems you are facing trying to make Kysely work in the browser or in deno? I think we should focus on those instead of creating a crapload of packages and extra work.

[steida]

Please correct me if I'm wrong, but I suppose all dialects can be part of one Kysely lib, only the code can't be coupled. The browser will not load modules that are not required/imported. All Kysely needs is different composition roots (per environment), as I see it.

[steida]

Migrations for environments without a file system would be nice. Migration is a TypeScript file that could be imported manually anyway. With such a design, Kysely could be used everywhere.

[sampbarrow]

Migrations for environments without a file system would be nice. Migration is a TypeScript file that could be imported manually anyway. With such a design, Kysely could be used everywhere.

This is how it works now, I just import a file with a TS object.

The proposed change would still allow for this, the object would just have to be wrapped in a MigrationProvider.

[koskimas]

Please correct me if I'm wrong, but I suppose all dialects can be part of one Kysely lib, only the code can't be coupled. The browser will not load modules that are not required/imported. All Kysely needs is different composition roots (per environment), as I see it.

That's already the case. Nothing gets loaded until it's used. pg or mysql etc. are only imported once the first query using those dialects is executed. If that's enough, there's nothing we need to do. And if this is the case, I could move fs.readdir and path.join back inside Migrator and only load them dynamically.

There are already a lot of circular dependencies in Kysely. But as I wrote, I think Kysely can be single lib without dynamic imports.

I meant a package requiring another package requiring the first one again. You update one, you need to update the other. And then kysely tests need to import the dialect package again.

Migrations for environments without a file system would be nice. Migration is a TypeScript file that could be imported manually anyway. With such a design, Kysely could be used everywhere.

You can implement your own MigrationProvider. It has one method getMigrations that should return the migrations. Easy as that.

[koskimas]

Here's a Migrator with a custom migration provider

const migrator = new Migrator({
  db, 
  provider: {
    getMigrations(): Promise<Record<string, Migration>> {
      // Do whatevery you want to get a bunch of objects that
      // implement the `Migration` interface
    }
  }
})

[steida]

If that's enough, there's nothing we need to do.

Kysely works but also warns:

./node_modules/kysely/dist/esm/util/dynamic-import.js
Critical dependency: the request of a dependency is an expression

dynamic-import.js is incompatible with Webpack

What does critical dependency the request of a dependency is an expression mean? Webpack - Critical dependency: the request of a dependency is an expression. ... When a library uses variables or expressions in a require call (such as require('' + 'nodent') in these lines of ajv ), Webpack cannot resolve them statically and imports the entire package.

I would use a composition root instead of a dynamic import. Otherwise, it can't work without warnings I suppose.

[koskimas]

Why do you need a "composition root"? Just import the files you use from their folders and never use import ... from 'kysely'.

For example import { Kysely} from 'kysely/dist/cjs/kysely'

[steida]

Composition root imports eagerly so dynamic import would not be required. I am using Kysely as described and Next.js Webpack warns. That's all.

I think I or @midwesterntechnology should provide a repo with Kysely node-less usage. I will try to prepare something.

Feel free to postpone this issue because it's not a show-stopper.

[koskimas]

Yes, I know the basics of javascript. No need to constantly teach them to me.

What I meant was NOT importing the composition file that eagerly loads stuff, but instead importing the separate files individually:

import { Kysely} from 'kysely/dist/cjs/kysely'

The file in the above example IS NOT an entry point. It only contains Kysely class's code and imports it's dependencies, which don't include Migrator or any of the dialects that run dynamic imports.

We could in theory add different entry points like

import { Kysely} from 'kysely/dist/cjs/browser'

but is that really needed?

[steida]

It's probably only a problem for my AbsurdSQL dialect. And for anyone who will write any dialect for some node-less environment, I guess. If I understand it correctly, imports could be rewritten. So maybe it's an issue only for dialects creators.

import {
  CompiledQuery,
  DatabaseConnection,
  DatabaseIntrospector,
  DatabaseMetadata,
  DatabaseMetadataOptions,
  DefaultQueryCompiler,
  Dialect,
  DialectAdapter,
  Driver,
  Kysely,
  MIGRATION_LOCK_TABLE,
  MIGRATION_TABLE,
  QueryCompiler,
} from 'kysely';

[koskimas]

Why is it a problem in that example? Because there are so many imports you'd have to find direct paths for? None of those would import anything node-related or any of the external packages or anything that uses dynamic imports, if you import them separately.

[koskimas]

In 0.15.3 there's now an experimental index-nodeless entry point you should be able to use. It exports everything except the files that depend on node or have dynamic node imports in them (or import the dynamic-imports.js module)

So please try this:

import {
  CompiledQuery,
  DatabaseConnection,
  DatabaseIntrospector,
  DatabaseMetadata,
  DatabaseMetadataOptions,
  DefaultQueryCompiler,
  Dialect,
  DialectAdapter,
  Driver,
  Kysely,
  MIGRATION_LOCK_TABLE,
  MIGRATION_TABLE,
  QueryCompiler,
} from 'kysely/dist/cjs/index-nodeless.js'; // <-- `.js` at the end is required!

Dynamic imports are still needed for node users (which is the vast majority of users). I want to provide the node users a single entry point but I don't want to force them to install all database drivers. That leads to dynamic imports.

But the files that run dynamic imports and import the dynamic-imports.js file, are not included in the index-nodeless entry point.

@steida Please just try if this works and doesn't produce warnings anymore.

[steida]

I tested both dev and production builds, and warnings are gone! A build is slightly bigger (from 169 kB to 178 kB gzipped), but that's because CommonJS is used instead of ES Modules.

Thank you for the fix and explanation of why dynamic imports are used.

[koskimas]

You can import the esm version from kysely/dist/esm/index-nodeless.js.

I have this in package.json

{
  "exports": {
    ".": {
      "import": "./dist/esm/index.js",
      "require": "./dist/cjs/index.js"
    },
    "./dist/cjs/index-nodeless.js": "./dist/cjs/index-nodeless.js",
    "./dist/esm/index-nodeless.js": "./dist/esm/index-nodeless.js"
  },
}

I should be able to do this

{
  "exports": {
    ".": {
      "import": "./dist/esm/index.js",
      "require": "./dist/cjs/index.js"
    },
    "./nodeless": {
      "import": "./dist/esm/index-nodeless.js",
      "require": "./dist/cjs/index-nodeless.js"
    },
  },
}

but at least vscode wasn't parsing that and said you can't import from kysely/nodeless. Maybe in the future we can update that and then you automatically get the esm version if you are using esm.

[koskimas]

I would love to get this working in the Deno runtime. I've tried this using their node compatibility mode and using esbuilder tools like https://www.skypack.dev/view/kysely but could not get it to work.

@barthuijgen Could you try if this works:

import kysely from 'https://cdn.skypack.dev/kysely/dist/esm/index-nodeless.js'

or this

import kysely from 'https://cdn.skypack.dev/kysely/dist/cjs/index-nodeless.js'

[barthuijgen]

I tried both, but just by opening the urls in browser you can already see it's having some issues with it.

the ems one gives this output

console.warn("[Package Error] \"kysely@v0.16.0\" could not be built. \n[1/5] Verifying package is valid…\n[2/5] Installing dependencies from npm…\n[3/5] Building package using esinstall…\nRunning esinstall...\n
Failed to load node_modules/kysely/dist/esm/migration/migrator.js\n  Unexpected token (151:10) in kysely/dist/esm/migration/migrator.js\nInstall failed.\nInstall failed.");
throw new Error("[Package Error] \"kysely@v0.16.0\" could not be built. ");
export default null;

The cjs version seems to be linking to the same build output. I'm not sure if skypack is trying to build it from root.

I also tried cloning master and running npm run build and importing kysely/dist/esm/index-nodeless.js this does run, but it does not pick up the .d.ts files. More info about how Deno loads them can be found here: https://deno.land/manual@v1.9.2/typescript/types

To compete the test I had to do the following steps to get it working

• Wrote a script to prepend /// <reference types="./${baseName}" />\n to the esm output
• Add .js to the imports in src/operation-node/constraint-node.ts
• Remove references to Buffer in /src/util/object-utils.ts since they cause a node reference

With this the output could be imported in Deno correctly and works, well I didn't have a driver, but it compiled and ran without any type issues.

Reference: https://github.com/koskimas/kysely/pull/50

Edit: I'm not suggesting to merge that MR, it's just for reference, skypack uses headers to make the .d.ts files work, this is just for local testing. I'm not sure why skypack is failing here, i'll dig a little deeper in how they build the output.

Edit2: found some more info about skypack exports here https://docs.skypack.dev/package-authors/package-checks#export-map it seems they suggest adding a key like "deno" in the exports map in package.json, where the current key is ./dist/esm/index-nodeless.js. Can't find any details on how the url should be formed to use certain exports though.

[koskimas]

it seems they suggest adding a key like "deno" in the exports map in package.json, where the current key is ./dist/esm/index-nodeless.js. Can't find any details on how the url should be formed to use certain exports though.

I tried adding "./nodeless" to the exports map, but at least vscode wasn't able to parse it and didn't allow import { ... } from 'kysely/nodeless'

[koskimas]

@barthuijgen 0.16.3 has /// <reference types="..." /> directives in the .js files under the esm folder.

[barthuijgen]

Hey, thanks for picking up any suggestions so quickly that's awesome.

I see the types are picked up immediately now when using jsDelivr.

import { Kysely } from "https://cdn.jsdelivr.net/npm/kysely/dist/esm/index-nodeless.js";

The only thing is still the node reference in object-utils.d.ts see https://cdn.jsdelivr.net/npm/kysely/dist/esm/util/object-utils.d.ts, Deno won't compile with this by default.

I honestly don't know how to solve this nicely. I tried googling around a bit for other projects that try to target both node and deno (and web) but most that I find are written for Deno first, or for Node and use Deno's compatability tools, not any smooth inbetween. I'm sure there is some way to do this all nicely, but we will have to experiment more and try to find the right way it seems. I'll get back to you if I find something 👍

[koskimas]

@barthuijgen The references to Buffer should now be gone in 0.16.5. The constructor is still used conditionally in the javascript code in one place, but the types no longer refer to it in any way. Let me know if the single reference still somehow messes things up.