Improve Support for Liftoff's Preloaders (#3613)

bin/cli.js

@@ -12,7 +12,6 @@ const { promisify } = require('util');
 const cliPkg = require('../package');
 const {
   mkConfigObj,
-  resolveKnexFilePath,
   resolveEnvironmentConfig,
   exit,
   success,
@@ -34,6 +33,21 @@ function initKnex(env, opts) {
     // enable esm interop via 'esm' module
     require = require('esm')(module);
   }
+
+  env.configuration = env.configPath
+    ? require(env.configPath)
+    : mkConfigObj(opts);
+
+  // 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;
+
+    // TODO: Should this property be documented somewhere?
+    env.configuration.ext = path.extname(p).replace('.', '');
+  }
+
   checkLocalModule(env);
   if (process.cwd() !== env.cwd) {
     process.chdir(env.cwd);
@@ -43,37 +57,6 @@ function initKnex(env, opts) {
     );
   }
 
-  if (!opts.knexfile) {
-    const configurationPath = resolveKnexFilePath();
-    const configuration = configurationPath
-      ? require(configurationPath.path)
-      : undefined;
-
-    env.configuration = configuration || mkConfigObj(opts);
-    if (!env.configuration.ext && configurationPath) {
-      env.configuration.ext = configurationPath.extension;
-    }
-  }
-  // If knexfile is specified
-  else {
-    const resolvedKnexfilePath = path.resolve(opts.knexfile);
-    const knexfileDir = path.dirname(resolvedKnexfilePath);
-    process.chdir(knexfileDir);
-    env.configuration = require(resolvedKnexfilePath);
-
-    if (!env.configuration) {
-      exit(
-        'Knexfile not found. Specify a path with --knexfile or pass --client and --connection params in commandline'
-      );
-    }
-
-    if (!env.configuration.ext) {
-      env.configuration.ext = path
-        .extname(resolvedKnexfilePath)
-        .replace('.', '');
-    }
-  }
-
   const resolvedConfig = resolveEnvironmentConfig(opts, env.configuration);
   const knex = require(env.modulePath);
   return knex(resolvedConfig);
@@ -368,11 +351,21 @@ cli.on('requireFail', function(name) {
   console.log(color.red('Failed to load external module'), color.magenta(name));
 });
 
+// FYI: The handling for the `--cwd` and `--knexfile` arguments is a bit strange,
+//      but we decided to retain the behavior for backwards-compatibility.  In
+//      particular: if `--knexfile` is a relative path, then it will be resolved
+//      relative to `--cwd` instead of the shell's CWD.
+//
+//      So, the easiest way to replicate this behavior is to have the CLI change
+//      its CWD to `--cwd` immediately before initializing everything else.  This
+//      ensures that Liftoff will then resolve the path to `--knexfile` correctly.
+if (argv.cwd) {
+  process.chdir(argv.cwd);
+}
+
 cli.launch(
   {
-    cwd: argv.cwd,
-    knexfile: argv.knexfile,
-    knexpath: argv.knexpath,
+    configPath: argv.knexfile,
     require: argv.require,
     completion: argv.completion,
   },

bin/utils/cli-config-utils.js

@@ -8,9 +8,8 @@ const argv = require('getopts')(process.argv.slice(2));
 
 function mkConfigObj(opts) {
   if (!opts.client) {
-    const path = resolveDefaultKnexfilePath();
     throw new Error(
-      `No default configuration file '${path}' found and no commandline connection parameters passed`
+      `No configuration file found and no commandline connection parameters passed`
     );
   }
 
@@ -31,34 +30,6 @@ function mkConfigObj(opts) {
   };
 }
 
-function resolveKnexFilePath() {
-  const jsPath = resolveDefaultKnexfilePath('js');
-  if (fs.existsSync(jsPath)) {
-    return {
-      path: jsPath,
-      extension: 'js',
-    };
-  }
-
-  const tsPath = resolveDefaultKnexfilePath('ts');
-  if (fs.existsSync(tsPath)) {
-    return {
-      path: tsPath,
-      extension: 'ts',
-    };
-  }
-
-  console.warn(
-    `Failed to find configuration at default location of ${resolveDefaultKnexfilePath(
-      'js'
-    )}`
-  );
-}
-
-function resolveDefaultKnexfilePath(extension) {
-  return process.cwd() + `/knexfile.${extension}`;
-}
-
 function resolveEnvironmentConfig(opts, allConfigs) {
   const environment = opts.env || process.env.NODE_ENV || 'development';
   const result = allConfigs[environment] || allConfigs;
@@ -156,7 +127,6 @@ function getStubPath(configKey, env, opts) {
 
 module.exports = {
   mkConfigObj,
-  resolveKnexFilePath,
   resolveEnvironmentConfig,
   exit,
   success,

package.json

@@ -107,6 +107,7 @@
     "tap-spec": "^5.0.0",
     "tape": "^4.13.0",
     "toxiproxy-node-client": "^2.0.6",
+    "ts-node": "^8.5.4",
     "typescript": "3.7.4",
     "webpack-cli": "^3.3.10"
   },
@@ -189,7 +190,10 @@
     "lines": 84,
     "statements": 82,
     "functions": 83,
-    "branches": 69
+    "branches": 69,
+    "extension": [
+      ".js"
+    ]
   },
   "husky": {
     "hooks": {

test/cli/knexfile-test.spec.js

@@ -1,6 +1,8 @@
 'use strict';
 
 const path = require('path');
+const tildify = require('tildify');
+
 const { FileTestHelper, execCommand } = require('cli-testlab');
 
 const KNEX = path.normalize(__dirname + '/../../bin/cli.js');
@@ -24,35 +26,13 @@ describe('knexfile resolution', () => {
     process.env.KNEX_PATH = '../knex.js';
   });
 
-  it('Run migrations with knexfile passed', () => {
-    return execCommand(
-      `node ${KNEX} migrate:latest --knexfile=test/jake-util/knexfile/knexfile.js --knexpath=../knex.js`,
-      {
-        expectedOutput: 'Batch 1 run: 1 migrations',
-      }
-    );
-  });
-
-  it('Resolves migrations relatively to knexfile', () => {
-    return execCommand(
-      `node ${KNEX} migrate:latest --knexfile=test/jake-util/knexfile-relative/knexfile.js --knexpath=../knex.js`,
-      {
-        expectedOutput: 'Already up to date',
-      }
-    );
-  });
-
-  it('Throws informative error when no knexfile is found', () => {
-    return execCommand(`node ${KNEX} migrate:latest --knexpath=../knex.js`, {
-      expectedErrorMessage: 'No default configuration file',
-    });
-  });
-
-  it('Resolves default knexfile in working directory correctly', () => {
-    const path = process.cwd() + '/knexfile.js';
-    fileHelper.createFile(
-      path,
-      `
+  context('--cwd is NOT specified', function() {
+    context('and --knexfile is also NOT specified', function() {
+      it('Resolves default knexfile in working directory correctly', () => {
+        const path = process.cwd() + '/knexfile.js';
+        fileHelper.createFile(
+          path,
+          `
 module.exports = {
   client: 'sqlite3',
   connection: {
@@ -62,21 +42,103 @@ module.exports = {
     directory: __dirname + '/test//jake-util/knexfile_migrations',
   },
 };
-    `,
-      { isPathAbsolute: true }
-    );
+        `,
+          { isPathAbsolute: true }
+        );
 
-    return execCommand(`node ${KNEX} migrate:latest --knexpath=../knex.js`, {
-      expectedOutput: 'Batch 1 run: 1 migrations',
+        return execCommand(
+          `node ${KNEX} migrate:latest --knexpath=../knex.js`,
+          {
+            expectedOutput: 'Batch 1 run: 1 migrations',
+          }
+        );
+      });
+    });
+
+    context('but --knexfile is specified', function() {
+      it('Run migrations with knexfile passed', () => {
+        return execCommand(
+          `node ${KNEX} migrate:latest --knexfile=test/jake-util/knexfile/knexfile.js --knexpath=../knex.js`,
+          {
+            expectedOutput: 'Batch 1 run: 1 migrations',
+          }
+        );
+      });
+
+      it("changes the process's cwd to the directory that contains 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.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', () => {
+        return execCommand(
+          `node ${KNEX} migrate:latest --knexfile=test/jake-util/knexfile-relative/knexfile.js --knexpath=../knex.js`,
+          {
+            expectedOutput: 'Batch 1 run: 2 migrations',
+          }
+        );
+      });
+
+      it('Throws informative error when no knexfile is found', () => {
+        return execCommand(
+          `node ${KNEX} migrate:latest --knexpath=../knex.js`,
+          {
+            expectedErrorMessage: 'No configuration file found',
+          }
+        );
+      });
+    });
+  });
+
+  context('--cwd is specified', function() {
+    context('and --knexfile is also specified', function() {
+      context('and --knexfile is a relative path', function() {
+        it('resolves --knexfile relative to --cwd', function() {
+          return execCommand(
+            `node ${KNEX} migrate:latest --cwd=test/jake-util/knexfile --knexfile=knexfile.js`,
+            {
+              expectedOutput: 'Batch 1 run: 1 migrations',
+            }
+          );
+        });
+      });
+
+      context('and --knexfile is an absolute path', function() {
+        it('uses the indicated knexfile', function() {
+          // Notice: the Knexfile is using Typescript.  This means that Knex
+          // is pre-loading the appropriate Typescript modules before loading
+          // the Knexfile.
+          const knexfile = path.resolve(
+            'test/jake-util/knexfile-ts/custom-config.ts'
+          );
+          return execCommand(
+            `node ${KNEX} migrate:latest --cwd=test/jake-util/knexfile --knexfile=${knexfile}`,
+            {
+              expectedOutput: 'Batch 1 run: 4 migrations',
+            }
+          );
+        });
+      });
+    });
+
+    context('but --knexfile is NOT specified', function() {
+      it('resolves knexfile relative to the specified cwd', () => {
+        return execCommand(
+          `node ${KNEX} migrate:latest --cwd=test/jake-util/knexfile`,
+          {
+            expectedOutput: 'Batch 1 run: 1 migrations',
+          }
+        );
+      });
     });
   });
 });
-
-it('resolves knexfile correctly with cwd specified', () => {
-  return execCommand(
-    `node ${KNEX} migrate:latest --cwd=test/jake-util/knexfile --knexfile=knexfile.js`,
-    {
-      expectedOutput: 'Batch 1 run: 1 migrations',
-    }
-  );
-});

test/jake-util/knexfile-relative/knexfile_migrations/20200105205226_one.js

@@ -0,0 +1,10 @@
+
+exports.up = (knex) => {
+  return knex.schema.createTable('one', (table) => {
+    table.string('name');
+  });
+};
+
+exports.down = (knex) => {
+  return knex.schema.dropTable('one');
+};

test/jake-util/knexfile-relative/knexfile_migrations/20200105205320_two.js

@@ -0,0 +1,10 @@
+
+exports.up = (knex) => {
+  return knex.schema.createTable('two', (table) => {
+    table.string('name');
+  });
+};
+
+exports.down = (knex) => {
+  return knex.schema.dropTable('two');
+};

test/jake-util/knexfile-ts/custom-config.ts

@@ -0,0 +1,14 @@
+
+export const client = 'sqlite3';
+
+export const connection = {
+  filename: '../test.sqlite3',
+};
+
+export const migrations = {
+  directory: './knexfile_migrations',
+};
+
+export const seeds = {
+  directory: './knexfile_seeds',
+};

test/jake-util/knexfile-ts/knexfile_migrations/20200131112749_one.ts

@@ -0,0 +1,10 @@
+import * as Knex from "../../../../knex";
+
+
+export async function up(knex: Knex): Promise<any> {
+}
+
+
+export async function down(knex: Knex): Promise<any> {
+}
+

test/jake-util/knexfile-ts/knexfile_migrations/20200131112753_two.ts

@@ -0,0 +1,10 @@
+import * as Knex from "../../../../knex";
+
+
+export async function up(knex: Knex): Promise<any> {
+}
+
+
+export async function down(knex: Knex): Promise<any> {
+}
+

test/jake-util/knexfile-ts/knexfile_migrations/20200131112757_three.ts

@@ -0,0 +1,10 @@
+import * as Knex from "../../../../knex";
+
+
+export async function up(knex: Knex): Promise<any> {
+}
+
+
+export async function down(knex: Knex): Promise<any> {
+}
+

test/jake-util/knexfile-ts/knexfile_migrations/20200131112801_four.ts

@@ -0,0 +1,10 @@
+import * as Knex from "../../../../knex";
+
+
+export async function up(knex: Knex): Promise<any> {
+}
+
+
+export async function down(knex: Knex): Promise<any> {
+}
+