Skip to content
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

node-pg-migrate cannot load migrations that are in upstream-only directories #907

Open
leinelissen opened this issue Jul 22, 2022 · 1 comment
Labels
s: pending triage Pending Triage

Comments

@leinelissen
Copy link

Hi there,

I am currently working on a legacy codebase which implements node-pg-migrate. It basically does a directory search in a particular folder, and then passes those files one-by-one into the runner programmatic API. I ran into a really weird issue where a directory structure like this would succeed in running the migrations:

.
├ src/
│ └ migrations.js
└ migrations/
  ├ migration1.ts
  ├ migration2.ts
  └ ...

whereas the following directory structure would result in an Error: Cannot find module 'migrations/migrations1.ts'

.
├ migrations.js
└ migrations/
  ├ migration1.ts
  ├ migration2.ts
  └ ...

I dove down the rabbit hole and uncovered the following line that is at the basis of the issue:

require(path.relative(__dirname, filePath))

What it comes down is the following. As per the NodeJS module resolution algorithm, only specific cases are resolved to local files. When path.relative calculates a path that has to move downstream first before moving upstream (Example 1), the paths it produces are always prefixed with ../. In this case the module resolution algorithm will assume the imports are local files, resolve the paths and them import them. The issue comes with Example 2. If the path is straight upstream from the file itself, path.relative doesn't have to prefix the path with anything, thus resulting in a relative path like migrations/migrations1.ts. However, because the lack of prefix, the resolution algorithm will now recognise the import as a module import, which has many different rules for import, of which none is importing local files. Hence the import will fail.

There are two solutions for resolving these issues:

  1. Making sure that if the relative path is not prefixed with ../ or..\, to prefix it with ./ or .\ so Node knows to resolve the path as a local file
  2. Forgoing path.relative completely, and just directly inputting the absolute path, which Node will resolve as local file as well.

I don't necessarily see why path.relative was necessary in the first case, but I can imagine that the first solution is more backwards-compatible regarding edge cases. I would love to make a PR to fix this issue, but I need some input first as to which solution makes more sense.

Cheers!
Lei

@moltar
Copy link

moltar commented Oct 18, 2022

I think this is another issue that could benefit from #651

@Shinigami92 Shinigami92 added the s: pending triage Pending Triage label May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s: pending triage Pending Triage
Projects
None yet
Development

No branches or pull requests

3 participants