-
Notifications
You must be signed in to change notification settings - Fork 171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(runner): resolve migration file for esm #1207
Conversation
@aprendendofelipe could you do a review for this? |
0721729
to
9a3a8f8
Compare
cfdb4da
to
5bc0b7b
Compare
Okay... I need more time to find out what's going on 😕 but ran out of time today |
src/runner.ts
Outdated
const relativeFilePath = relative(directory, filePath); | ||
|
||
return ( | ||
globalThis.require?.(relativeFilePath) ?? (await import(relativeFilePath)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import
will not work with .ts
at runtime.
What do you think about #1206:
const filePath = resolve(options.dir, file);
const actions: MigrationBuilderActions =
extname(filePath) === '.sql'
? await migrateSqlFile(filePath)
: createRequire(resolve('noop.js'))(filePath);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, pls read all my comments in your other PR 🙂
But regarding .ts
files, I was shocked a bit to discover what's really going on here 🤔 (I just took over maintenance and I'm not the original creator)
When we will go full esm only in v9, I thought that we will import the migration file and consume it as a real js file. But now there is potential also .ts files at runtime? 👀
Either we could try to compile them at runtime, as we are aware that typescript is installed as peer dependency anyway (cause the user is already using .ts files) or ... I dont know 🤔 I'm not so deep into that right now, I need to build up some knowledge...
FFR: |
Co-authored-by: Felipe Barso <aprendendofelipe@users.noreply.github.com>
@aprendendofelipe assuming that
Can we find a solution that works, so that the tests are not failing (without touching them obviously) |
extracted from #1206
__dirname
andpg.Client
#1206Edit: behavior changed to tryout using process cwd