K
Join ServerKysely
help
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\kysely@0.24.2\node_modules\kysely\dist\esm\migration\file-migration-provider.js 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.
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 ESModulesWhen 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:'Well there it it. Just add
file:// to the beginning. Did you even read the error 😄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
}
}
}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
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
Honestly I have no idea
I think it fails when trying to read the directory for the filenames inside
Oh, damn. that's true
Why the hell is there a difference like this between windows and other platforms in node 😲
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
Where does the
filePathToURL function come from?Or is it your own helper?
from the
'url' node packageimport { pathToFileURL} from 'url'wrong order of words
sorry
Crap.. Can't use that since there should be no references to node in Kysely
Can you use the web API URL?
I'll fix that somehow. In the meantime you can just copy paste the
FileMigrationProvider and use your own modified versionYeah I already did that. Trying to think of a way to fix it without depending on Node
Can you use the web API URL?Yes, but I also need to check if we're on windows. That's node-specific
how about allowing to pass a custom
importor fileparser
The whole
FileMigrationProvider API is already annoying. It takes the node dependencies as argumentsYeah 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,
});filePathParser would be a better name
Is wsl2 not an option?
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 constructorCan you provide a repository I could clone and test on my windows machine?
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
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?
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
}
}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
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.
We could extend and override the file getting part with a
Windows classand 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 denoMe 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
actually deno is fine: https://deno.land/manual@v1.17.2/npm_nodejs/compatibility_mode
What about having the migrator in a separate package for each environment? Although I think it would be annoying to mantain it that way
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>I run into the problem using ts-node, so I don't think that would be a solution.
That's to avoid the bundler issues. don't remember dynamic imports causing any problems with ts-node/tsx
Oh
Ok that makes sense
Message Not Public
Sign In & Join Server To View
Just making the changes to the package code in my windows installation. That is the best I could do
2 Messages Not Public
Sign In & Join Server To View
Yeah I wrote it down wrong in the example code
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
Message Not Public
Sign In & Join Server To View
I think a
kysely-migrations-windows could be a good ideaMessage Not Public
Sign In & Join Server To View
afaik, it should work with deno out the box.