You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
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:
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
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
The text was updated successfully, but these errors were encountered:
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:
whereas the following directory structure would result in an
Error: Cannot find module 'migrations/migrations1.ts'
I dove down the rabbit hole and uncovered the following line that is at the basis of the issue:
node-pg-migrate/src/runner.ts
Line 35 in 9331f6f
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 likemigrations/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:
../
or..\
, to prefix it with./
or.\
so Node knows to resolve the path as a local filepath.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
The text was updated successfully, but these errors were encountered: