Incorrect reference to __dirname when compiling to esmodules
PatKayongo opened this issue · 4 comments
Describe the bug
When running the migrations using the Programmatic API, the node-pg-migrate
source has a reference to __dirname
which is not available when using ES modules.
Steps to reproduce
When running the migrations using the Programmatic API using ES modules as follows:
import nodePostgres from 'pg';
import migrationRunner from 'node-pg-migrate';
import { fileURLToPath } from 'url';
import path from 'path';
const { Pool } = nodePostgres;
export const initialiseDatabase = async (databaseConfig) => {
const pool = new Pool({
host: databaseConfig.host,
database: databaseConfig.database,
port: databaseConfig.port,
password: databaseConfig.password,
user: databaseConfig.username,
ssl: { rejectUnauthorized: false },
});
const migrationsPath = path.join(path.dirname(fileURLToPath(import.meta.url)), 'migrations');
const migrationClient = await pool.connect();
try {
await migrationRunner({
dbClient: migrationClient,
migrationsTable: 'database_migrations',
dir: migrationsPath,
direction: 'up',
noLock: true,
});
migrationClient.release();
}
catch (error) {
migrationClient.release();
throw error;
}
return pool;
};
The following error is returned:
Error: Can't get migration files: ReferenceError: __dirname is not defined
Looking at the source file, it looks like there is a reference to __dirname, which is only accessible using CommonJS modules.
Logs
Error: Can't get migration files: ReferenceError: __dirname is not defined
at file:///path/to/project/node_modules/node-pg-migrate/dist/esm/index.mjs:2579:115
at Array.map ()
at loadMigrations (file:///path/to/project/node_modules/node-pg-migrate/dist/esm/index.mjs:2577:13)
at async Promise.all (index 0)
at async runner (file:///path/to/project/node_modules/node-pg-migrate/dist/esm/index.mjs:2763:36)
at async initialiseDatabase (file:///path/to/project/dist/infrastructure/postgres/index.mjs:19:9)
at async file:///path/to/project/dist/index.mjs:13:26
at loadMigrations (file:///path/to/project/node_modules/node-pg-migrate/dist/esm/index.mjs:2601:11)
at async Promise.all (index 0)
at async runner (file:///path/to/project/node_modules/node-pg-migrate/dist/esm/index.mjs:2763:36)
at async initialiseDatabase (file:///path/to/project/dist/infrastructure/postgres/index.mjs:19:9)
at async file:///path/to/project/dist/index.mjs:13:26
System Info
System:
OS: macOS 14.4.1
CPU: (10) arm64 Apple M1 Pro
Memory: 59.11 MB / 16.00 GB
Shell: 5.9 - /bin/zsh
Binaries:
Node: 20.11.1 - ~/.nvm/versions/node/v20.11.1/bin/node
Yarn: 3.2.0 - ~/.nvm/versions/node/v20.11.1/bin/yarn
npm: 10.2.4 - ~/.nvm/versions/node/v20.11.1/bin/npm
Used Module System
esm
This should be catchable with a lint rule
This should be catchable with a lint rule
What do you mean?
__dirname
is legit in node-pg-migrate's codebase cause it was historically fully written only with cjs in mind.
Everything here is valid and over in #1168 we are already working on a fix.
However, we need to ensure compatibility to cjs and esm for the same time to ensure a smooth migration between major versions.
Ah right, since it needs to support both, then just banning one style or the other doesn’t make as much sense.
That said as a pattern I’ve had some success in other contexts using linting as a detection tool - ban both patterns, but then selectively add lint ignore comments with explanations ensure that the places where the library does need to use them have been exhaustively audited.
That lets the tooling guide us to detect missed cases and avoid regressions.
Just a note, I do not believe #1206 will resolve this issue. Please see my evaluation of the experimental release which was intended to resolve #1168 (which was somewhat subsumed by #1187).