Skip to content

Commit

Permalink
Fix: Knex CLI calls process.chdir() before opening Knexfile (#3661)
Browse files Browse the repository at this point in the history
  • Loading branch information
briandamaged committed Feb 13, 2020
1 parent 88d832c commit 2c206a8
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 14 deletions.
29 changes: 15 additions & 14 deletions bin/cli.js
Expand Up @@ -28,26 +28,18 @@ const fsPromised = {
writeFile: promisify(fs.writeFile),
};

function initKnex(env, opts) {
if (opts.esm) {
// enable esm interop via 'esm' module
require = require('esm')(module);
}

env.configuration = env.configPath
? require(env.configPath)
: mkConfigObj(opts);
function openKnexfile(configPath) {
const config = require(configPath);

// FYI: By default, the extension for the migration files is inferred
// from the knexfile's extension. So, the following lines are in
// place for backwards compatibility purposes.
if (!env.configuration.ext) {
const p = env.configPath || opts.knexpath;
config.ext = config.ext || path.extname(configPath).replace('.', '');

// TODO: Should this property be documented somewhere?
env.configuration.ext = path.extname(p).replace('.', '');
}
return config;
}

function initKnex(env, opts) {
checkLocalModule(env);
if (process.cwd() !== env.cwd) {
process.chdir(env.cwd);
Expand All @@ -57,6 +49,15 @@ function initKnex(env, opts) {
);
}

if (opts.esm) {
// enable esm interop via 'esm' module
require = require('esm')(module);
}

env.configuration = env.configPath
? openKnexfile(env.configPath)
: mkConfigObj(opts);

const resolvedConfig = resolveEnvironmentConfig(opts, env.configuration);
const knex = require(env.modulePath);
return knex(resolvedConfig);
Expand Down
21 changes: 21 additions & 0 deletions test/cli/knexfile-test.spec.js
Expand Up @@ -77,6 +77,27 @@ module.exports = {
);
});

// This addresses the issue that was reported here:
//
// https://github.com/knex/knex/issues/3660
//
context(
'and the knexfile itself resolves paths relative to process.cwd()',
function() {
it("changes the process's cwd to the directory that contains the knexfile before opening the knexfile", () => {
const knexfile = 'test/jake-util/knexfile-relative/knexfile.js';
const expectedCWD = tildify(path.resolve(path.dirname(knexfile)));

return execCommand(
`node ${KNEX} migrate:latest --knexfile=test/jake-util/knexfile-relative/knexfile-with-resolve.js --knexpath=../knex.js`,
{
expectedOutput: `Working directory changed to ${expectedCWD}`,
}
);
});
}
);

// FYI: This is only true because the Knex CLI changes the CWD to
// the directory of the knexfile.
it('Resolves migrations relatively to knexfile', () => {
Expand Down
24 changes: 24 additions & 0 deletions test/jake-util/knexfile-relative/knexfile-with-resolve.js
@@ -0,0 +1,24 @@
// This knexfile recreates the issue that was identified here:
//
// https://github.com/knex/knex/issues/3660
//

const path = require('path');

// Notice: this path will resolve relative to `process.cwd()` .
// This will cause a problem if the Knex CLI opens the knexfile
// before changing the cwd.
const MIGRATIONS_DIR = path.resolve('knexfile_migrations');

module.exports = {
client: 'sqlite3',
connection: {
filename: __dirname + '/../test.sqlite3',
},
migrations: {
directory: MIGRATIONS_DIR,
},
seeds: {
directory: './knexfile_seeds',
},
};

0 comments on commit 2c206a8

Please sign in to comment.