Topics

Kysely•2y ago

Meithz (Razioner)

Trouble running migrations

I am trying to use Kysely on a svelte-kit project, I set up the migration script like the example shown in the docs, but when trying to run the script with ts-node --esm "path/to/script" I receive failed to migrate

 CustomError: ERR_INVALID_MODULE_SPECIFIER src\lib\server\db\migrations\540540540_create_db.ts is not a valid package name E:\Code\Web\portfolio-projects\spotify-cp\node_modules\.pnpm\[email protected]\node_modules\kysely\dist\esm\migration\file-migration-provider.js

 CustomError: ERR_INVALID_MODULE_SPECIFIER src\lib\server\db\migrations\540540540_create_db.ts is not a valid package name E:\Code\Web\portfolio-projects\spotify-cp\node_modules\.pnpm\[email protected]\node_modules\kysely\dist\esm\migration\file-migration-provider.js

I tried changin the migration file name, making it a JS file instead of TS but still get the same error, I am not sure what I am doing wrong, the scripts are the same as the examples in the docs.

44 Replies

koskimas•2y ago

Does it work without the --esm flag? Oh, actually this could be just a case of relative paths instead of absolute. You need to use an absolute path to the migrations folder.

Meithz (Razioner)OP•2y ago

I check using absolute path It doesn't work without the --esm flag but the error is from ts-node, because the project is using ESModules When trying with the absolute path I get an error, but it is about ESM path resolution in Windows

[ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only URLs with a scheme in: file and data are supported by the default ESM loader. On Windows, absolute paths must be valid file:// URLs. Received protocol 'e:'

[ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only URLs with a scheme in: file and data are supported by the default ESM loader. On Windows, absolute paths must be valid file:// URLs. Received protocol 'e:'

koskimas•2y ago

Well there it it. Just add file:// to the beginning. Did you even read the error 😄

Meithz (Razioner)OP•2y ago

Still doesn't like it I run across this error before with another package Okey I went back to check the error I had come across in the other package, when using import() on Windows it requires the path to be a file URL, but I cannot pass it as that since I need to pass the directory, not the actual file. so when the migrator tries to import the migration file, there it should change it to a file URL.

 for (const fileName of files) {
      if (
        fileName.endsWith('.js') ||
        (fileName.endsWith('.ts') && !fileName.endsWith('.d.ts')) ||
        fileName.endsWith('.mjs') ||
        (fileName.endsWith('.mts') && !fileName.endsWith('.d.mts'))
      ) {
        // Doing something like this fixes it in Windows:
        const migrationPath = filePathToURL(this.#props.path.join(
            this.#props.migrationFolder,
            fileName
          )).pathname
        
        const migration = await import( migrationPath)
        
        const migrationKey = fileName.substring(0, fileName.lastIndexOf('.'))

        // Handle esModuleInterop export's `default` prop...
        if (isMigration(migration?.default)) {
          migrations[migrationKey] = migration.default
        } else if (isMigration(migration)) {
          migrations[migrationKey] = migration
        }
      }
    }

 for (const fileName of files) {
      if (
        fileName.endsWith('.js') ||
        (fileName.endsWith('.ts') && !fileName.endsWith('.d.ts')) ||
        fileName.endsWith('.mjs') ||
        (fileName.endsWith('.mts') && !fileName.endsWith('.d.mts'))
      ) {
        // Doing something like this fixes it in Windows:
        const migrationPath = filePathToURL(this.#props.path.join(
            this.#props.migrationFolder,
            fileName
          )).pathname
        
        const migration = await import( migrationPath)
        
        const migrationKey = fileName.substring(0, fileName.lastIndexOf('.'))

        // Handle esModuleInterop export's `default` prop...
        if (isMigration(migration?.default)) {
          migrations[migrationKey] = migration.default
        } else if (isMigration(migration)) {
          migrations[migrationKey] = migration
        }
      }
    }

Doing that when the migrator attempts to import the file fixes the problem However I don't know how this would work on other operating systems

koskimas•2y ago

I don't understand. Why can't you just provide the beginning of the URL as the "folder" and then it will be suffixed with the file name --> still a valid URL

Meithz (Razioner)OP•2y ago

Honestly I have no idea I think it fails when trying to read the directory for the filenames inside

koskimas•2y ago

Oh, damn. that's true Why the hell is there a difference like this between windows and other platforms in node 😲

Meithz (Razioner)OP•2y ago

I wondered the same thing the last time I had this problem, and it so specific to a small amount of users, because it is the combination of ESM + Windows that it is hard to locate They should at least make a very visible warning about this behaviour

koskimas•2y ago

Where does the filePathToURL function come from? Or is it your own helper?

Meithz (Razioner)OP•2y ago

from the 'url' node package import { pathToFileURL} from 'url' wrong order of words sorry

koskimas•2y ago

Crap.. Can't use that since there should be no references to node in Kysely

Meithz (Razioner)OP•2y ago

Can you use the web API URL?

koskimas•2y ago

I'll fix that somehow. In the meantime you can just copy paste the FileMigrationProvider and use your own modified version

Meithz (Razioner)OP•2y ago

Yeah I already did that. Trying to think of a way to fix it without depending on Node

koskimas•2y ago

Can you use the web API URL?

Yes, but I also need to check if we're on windows. That's node-specific

Meithz (Razioner)OP•2y ago

how about allowing to pass a custom import or fileparser

koskimas•2y ago

The whole FileMigrationProvider API is already annoying. It takes the node dependencies as arguments

Meithz (Razioner)OP•2y ago

Yeah I noticed that, so how about:

    const migrator = new Migrator({
        db,
        provider: new FileMigrationProvider({
            fs,
            path,
            migrationFolder: 'E:/Code/Web/portfolio-projects/spotify-cp/src/lib/server/db/migrations'
        }),
        filePathParser: (path) => pathToFileURL(path).pathname,
    });

    const migrator = new Migrator({
        db,
        provider: new FileMigrationProvider({
            fs,
            path,
            migrationFolder: 'E:/Code/Web/portfolio-projects/spotify-cp/src/lib/server/db/migrations'
        }),
        filePathParser: (path) => pathToFileURL(path).pathname,
    });

filePathParser would be a better name

Igal•2y ago

Is wsl2 not an option?

Meithz (Razioner)OP•2y ago

I guess it could be, haven't really tried to use it. But I think it would be better if it worked directly on windows, or if there was a easy way to make it work, like passing the filePathParser or something similar to the constructor

Igal•2y ago

Can you provide a repository I could clone and test on my windows machine?

Meithz (Razioner)OP•2y ago

I will try to upload it tonight, you should be able to reproduce it just with a starter svelte-kit project in case you want to try it now

Meithz (Razioner)OP•2y ago

https://github.com/FranciscoMessina/spotify-collaborative-playlists

GitHub

GitHub - FranciscoMessina/spotify-collaborative-playlists

Contribute to FranciscoMessina/spotify-collaborative-playlists development by creating an account on GitHub.

Igal•2y ago

I'll look at it today, thanks This is how its done in nuxt-cookie-control https://github.com/dargmuesli/nuxt-cookie-control/pull/57/files#diff-030fc083b2cbf5cf008cfc0c49bb4f1b8d97ac07f93a291d068d81b4d1416f70R113-R114 so maybe something like this should work?

  async getMigrations(): Promise<Record<string, Migration>> {
    const migrations: Record<string, Migration> = {}
    const files = await this.#props.fs.readdir(this.#props.migrationFolder)

    for (const fileName of files) {
      if (
        fileName.endsWith('.js') ||
        (fileName.endsWith('.ts') && !fileName.endsWith('.d.ts')) ||
        fileName.endsWith('.mjs') ||
        (fileName.endsWith('.mts') && !fileName.endsWith('.d.mts'))
      ) {
        const filePath = path.resolve(this.#props.migrationFolder, fileName)
        const migrationPath =
          os.platform() === 'win32'
            ? url.pathToFileURL(filePath).href
            : filePath
        const migration = await import(migrationPath)
        const migrationKey = fileName.substring(0, fileName.lastIndexOf('.'))

        // Handle esModuleInterop export's `default` prop...
        if (isMigration(migration?.default)) {
          migrations[migrationKey] = migration.default
        } else if (isMigration(migration)) {
          migrations[migrationKey] = migration
        }
      }
    }

    return migrations
  }
}

  async getMigrations(): Promise<Record<string, Migration>> {
    const migrations: Record<string, Migration> = {}
    const files = await this.#props.fs.readdir(this.#props.migrationFolder)

    for (const fileName of files) {
      if (
        fileName.endsWith('.js') ||
        (fileName.endsWith('.ts') && !fileName.endsWith('.d.ts')) ||
        fileName.endsWith('.mjs') ||
        (fileName.endsWith('.mts') && !fileName.endsWith('.d.mts'))
      ) {
        const filePath = path.resolve(this.#props.migrationFolder, fileName)
        const migrationPath =
          os.platform() === 'win32'
            ? url.pathToFileURL(filePath).href
            : filePath
        const migration = await import(migrationPath)
        const migrationKey = fileName.substring(0, fileName.lastIndexOf('.'))

        // Handle esModuleInterop export's `default` prop...
        if (isMigration(migration?.default)) {
          migrations[migrationKey] = migration.default
        } else if (isMigration(migration)) {
          migrations[migrationKey] = migration
        }
      }
    }

    return migrations
  }
}

I managed to patch-package kysely in your repo and reached a further point in Migrator flow. Its painful to run kysely's tests for windows with docker and everything, might do it tomorrow

Meithz (Razioner)OP•2y ago

Yes, that way looks like it would work, but it is dependant on Node , and @koskimas said that it shouldn't have any dependencies in node. I also patched my kysely installation to make it work, without actually checking for the Os since I knew I was on windows.

Igal•2y ago

We could extend and override the file getting part with a Windows class and a Deno class? I don't like the part where the consumer has to pass fs and path. It probably doesn't make sense in deno

koskimas•2y ago

Me neither, but the other option was to not provide the whole thing We can't import node-specific stuff or the build will break on non-node environments Neither can we import Deno or Windows-specific stuff Even dynamic imports cause problems with people's bunders

Igal•2y ago

actually deno is fine: https://deno.land/[email protected]/npm_nodejs/compatibility_mode

Deno

Node.js compatibility mode | Manual | Deno

Starting with v1.15 Deno provides Node compatiblity mode that makes it possible to run a subset of programs authored for Node.js directly in Deno. Compatiblity mode can be activated by passing `--com

Meithz (Razioner)OP•2y ago

What about having the migrator in a separate package for each environment? Although I think it would be annoying to mantain it that way

Igal•2y ago

Maybe we should recommend not bundling migrations? Recommend ts-node / tsx for running migrations instead? Also, quoting the first paragraph in the README file:

Mainly developed for node.js but also runs on deno and in the browser.

I wonder if we could, by default, just use node's stuff, and allow to override the file import function as a whole with an interface such as:

importMigrationFile(folderPath: string, filename: string): Promise<Migration>

importMigrationFile(folderPath: string, filename: string): Promise<Migration>

Meithz (Razioner)OP•2y ago

I run into the problem using ts-node, so I don't think that would be a solution.

Igal•2y ago

That's to avoid the bundler issues. don't remember dynamic imports causing any problems with ts-node/tsx

Meithz (Razioner)OP•2y ago

Oh Ok that makes sense

Unknown User•2y ago

Message Not Public

Sign In & Join Server To View

Meithz (Razioner)OP•2y ago

Just making the changes to the package code in my windows installation. That is the best I could do

Unknown User•2y ago

Message Not Public

Sign In & Join Server To View

Meithz (Razioner)OP•2y ago

Yeah I wrote it down wrong in the example code

Igal•2y ago

Maybe this shouldn't be dynamically imported? Maybe CLI could create migration files (empty up & down) and import them where we need to iterate over them

Unknown User•2y ago

Message Not Public

Sign In & Join Server To View

Igal•2y ago

I think a kysely-migrations-windows could be a good idea

Unknown User•2y ago

Message Not Public

Sign In & Join Server To View

Igal•2y ago

afaik, it should work with deno out the box.

Igal•2y ago

https://deno.com/[email protected]/npm_nodejs/compatibility_mode

Deno

Node.js compatibility mode | Manual | Deno

Starting with v1.15 Deno provides Node compatiblity mode that can be activated by passing --compat flag in CLI.

mdthansil•2y ago

Use tsx instead of ts-node and use absolute path for migrationFolder

const migrator = new Migrator({
    db,
    provider: new FileMigrationProvider({
      fs,
      path,
      migrationFolder: path.resolve(path.join(__dirname, 'migrations')),
    }),
  });

const migrator = new Migrator({
    db,
    provider: new FileMigrationProvider({
      fs,
      path,
      migrationFolder: path.resolve(path.join(__dirname, 'migrations')),
    }),
  });

1.7KMembers

View on Discord

Want results from more Discord servers?

Add your server