Fix for ES Module detection using npm@7 (#4295) (#4296)

CHANGELOG.md

@@ -22,6 +22,7 @@
 - Fix issue with .withSchema usage with joins on a subquery #4267
 - Fix issue with schema usage with FROM clause contain QueryBuilder, function or Raw #4268
 - CLI: Address raised security warnings by dropping liftoff #4122
+- CLI: Fix an issue with npm@7 and ESM when `type` was set to `'module'` in `package.json` #4295
 - PostgreSQL: Add check to only create native enum once #3658
 - SQLite: Fix foreign key "on delete" when altering a table #4225
 - MySQL: Keep auto increment after rename #4266

lib/migrations/migrate/Migrator.js

@@ -80,12 +80,16 @@ class Migrator {
 
     const transactionForAll =
       !this.config.disableTransactions &&
-      !migrations.some((migration) => {
-        const migrationContents = this.config.migrationSource.getMigration(
-          migration
-        );
-        return !this._useTransaction(migrationContents);
-      });
+      !(
+        await Promise.all(
+          migrations.map(async (migration) => {
+            const migrationContents = await this.config.migrationSource.getMigration(
+              migration
+            );
+            return !this._useTransaction(migrationContents);
+          })
+        )
+      ).some((isTransactionUsed) => isTransactionUsed);
 
     if (transactionForAll) {
       return this.knex.transaction((trx) => {
@@ -133,17 +137,24 @@ class Migrator {
           migrationToRun = newMigrations[0];
         }
 
+        return {
+          migrationToRun,
+          useTransaction:
+            !migrationToRun ||
+            this._useTransaction(
+              this.config.migrationSource.getMigration(migrationToRun)
+            ),
+        };
+      })
+      .then(({ migrationToRun, useTransaction }) => {
         const migrationsToRun = [];
         if (migrationToRun) {
           migrationsToRun.push(migrationToRun);
         }
 
         const transactionForAll =
           !this.config.disableTransactions &&
-          (!migrationToRun ||
-            this._useTransaction(
-              this.config.migrationSource.getMigration(migrationToRun)
-            ));
+          (!migrationToRun || useTransaction);
 
         if (transactionForAll) {
           return this.knex.transaction((trx) => {

lib/migrations/util/import-file.js

@@ -1,14 +1,13 @@
-// When run via npm, we can leverage the injected environment variables to infer the import type
-const isTypeModule = process.env.npm_package_type === 'module';
+const isModuleType = require('./is-module-type');
 
 /**
  * imports 'mjs', else requires.
  * NOTE: require me late!
  * @param {string} filepath
  * @todo WARN on version 10 and '--experimental-modules' and '--esm'
  */
-module.exports = function importFile(filepath) {
-  return isTypeModule || filepath.endsWith('.mjs')
+module.exports = async function importFile(filepath) {
+  return (await isModuleType(filepath))
     ? import(require('url').pathToFileURL(filepath))
     : require(filepath);
 };

lib/migrations/util/is-module-type.js

@@ -0,0 +1,14 @@
+const { readFile } = require('./fs');
+
+module.exports = async function isModuleType(filepath) {
+  if (process.env.npm_package_json) {
+    // npm >= 7.0.0
+    const packageJson = JSON.parse(
+      await readFile(process.env.npm_package_json, 'utf-8')
+    );
+    if (packageJson.type === 'module') {
+      return true;
+    }
+  }
+  return process.env.npm_package_type === 'module' || filepath.endsWith('.mjs');
+};

test/db-less-test-suite.js

@@ -6,6 +6,7 @@ describe('Util Tests', function () {
   // Unit Tests for utilities.
   require('./unit/query/string');
   require('./unit/migrations/util/fs');
+  require('./unit/migrations/util/is-module-type');
   require('./unit/util/nanoid');
   require('./unit/util/save-async-stack');
   require('./unit/util/comma-no-paren-regex');

test/unit/migrations/util/is-module-type.js

@@ -0,0 +1,51 @@
+const path = require('path');
+
+const { expect } = require('chai');
+const isModuleType = require('../../../../lib/migrations/util/is-module-type.js');
+require('../../../util/chai-setup');
+
+describe('isModuleType', () => {
+  let originalEnv = {};
+
+  before(() => {
+    originalEnv = { ...process.env };
+    process.env = {};
+  });
+
+  after(() => {
+    process.env = { ...originalEnv };
+  });
+
+  beforeEach(() => {
+    delete process.env.npm_package_type;
+    delete process.env.npm_package_json;
+  });
+
+  it('should return true if the file is a .mjs file', async () => {
+    expect(await isModuleType('test.mjs')).to.be.true;
+  });
+
+  it('should return true if type=module with npm < 7.0.0', async () => {
+    process.env.npm_package_type = 'module';
+    expect(await isModuleType('test.js')).to.be.true;
+  });
+
+  it('should return false if type=commonjs with npm < 7.0.0', async () => {
+    process.env.npm_package_type = 'commonjs';
+    expect(await isModuleType('test.js')).to.be.false;
+  });
+
+  it('should return true if type=module with npm >= 7.0.0', async () => {
+    process.env.npm_package_json = path.normalize(
+      __dirname + '/test/package-module.json'
+    );
+    expect(await isModuleType('test.js')).to.be.true;
+  });
+
+  it('should return false if type=commonjs with npm >= 7.0.0', async () => {
+    process.env.npm_package_json = path.normalize(
+      __dirname + '/test/package-commonjs.json'
+    );
+    expect(await isModuleType('test.js')).to.be.false;
+  });
+});

test/unit/migrations/util/test/package-commonjs.json

@@ -0,0 +1,3 @@
+{
+    "type": "commonjs"
+}

test/unit/migrations/util/test/package-module.json

@@ -0,0 +1,3 @@
+{
+    "type": "module"
+}