Bug (Kinda) with ES Modules ESM - Solution Enclosed

[ryanlelek]

Hi, thanks for the module. Very useful and appreciated.
Opening this for discussion and to help others.

I have the pleasure/misfortune of dealing with ES Modules (using import) instead of CommonJS (require()).
Everything works fine normally, and Line 75 in lib/migration.js is giving issues.
75: return require(this.path);

To solve, I did some research and experimentation to find a simple solution.
Replacing:
return require(this.path);
with
return import(this.path);
solves it for ES Modules.

This can be accomplished with sed to swap out the line.
sed -i'.bak' 's/require(this.path);/import(this.path);/g' ./node_modules/umzug/lib/migration.js

Why it fails:

• require()-ing the migration file(s) does not make ES Modules happy
• instead of require() replacing with import() allows the migration file(s) to load as expected

I'm not sure how to support both CommonJS and ES Modules.
The issue likely stems from umzug being a module with its own package.json and namespace, then mixing in the migration files from our project which use the project's ES Module package.json and namespace.
More on ES Modules at this link if needed

Stats:

• Version: v2.3.0 (latest stable)
• Storage: SequelizeStorage
• Node v16.x.x
• ES Modules (in package.json by adding "type": "module",)

Any suggestions?
Thank you!

[mmkal]

Thanks for the report! I agree, given esmodules are the way of the future they should certainly be supported. My team is using umzug v3 in production with commonjs so I'm also not ready to stop supporting that. However it may be possible to allow both by adding an optional require constructor parameter, so that esmodules users can pass in via something like new Umzug({ require: createRequire(...) }). I am also planning on looking into tools like microbundle to output both commonjs and esmodules.

Note: it's unlikely anything will be back ported to 2.x - but after this I think it will be safe to release v3 stable (finally).

Edit: I realised after writing this that this is already supported in v3.x. You can just use a custom resolve function, as shown in this example. So @ryanlelek I'd recommend keeping your workaround until the stable release of v3.

[B4nan]

Edit: I realised after writing this that this is already supported in v3.x. You can just use a custom resolve function, as shown in this example. So @ryanlelek I'd recommend keeping your workaround until the stable release of v3.

Dynamic imports are async, they need to be awaited, which I believe is not done currently for the resolve callback.

[mmkal]

@B4nan that's right, the import needs to be moved down into the up and down methods themselves, which are expected to be async - as shown in the example linked above.

[B4nan]

Oh I totally missed that, thanks! Would be still cleaner to have the resolve supporting async functions out of box, but this is good enough :]

[joeldenning]

fwiw switching to import() instead of require() wouldn't break things for commonjs users. It's completely supported to use import() to load a cjs file

[mltsy]

FWIW: I am stuck currently with Umzug 2.x in a Next.js app, and tried using a customResolver to import rather than require the given file paths, but that produced some of the most mind-boggling errors I've ever seen, where it seemed to be trying to process markdown files that were not remotely related to the migrations/seeders as javascript, which obviously resulted in parsing errors. So I just wanted to post my solution for anyone who might be having similar struggles.

(Basically: forget umzug!)...
[EDIT: by which I mean, forget it for any particular use-case where it's causing conflicts with ESM, and not needed (e.g. running ESM migrations manually from a test script). Umzug is great in general, and handles many use-cases much more robustly than this!]

const seederPath = 'db/seeders'
const seedFiles = fs.readdirSync(seederPath);
const seederImports = seedFiles.map(async fileName => {
  return import(`${seederPath}/${fileName}`)
})

async function runSeeders() {
  for (const promise of seederImports) {
    const seeder = await promise
    await seeder.up(sequelize.getQueryInterface(), Sequelize)
  }
}

Warning: This will not record which migrations have been run anywhere, so if that is a requirement, you'll have to handle that separately.

[mltsy]

BUT, I am very much looking forward to the day umzug uses import instead of require, and everything just works (if @joeldenning's insight is correct) 😄

[mmkal]

When you say stuck on v2, what do you mean exactly? What's preventing updating to v3? There are examples for import(...) for v3 but v2 isn't going to receive any updates at this point.

[mltsy]

Oh thanks for the follow-up @mmkal! Just stuck currently due to using sequelize-cli, which is (AFAIK) not compatible with 3.x yet.

Also, I didn't mean to sound as if I'm against using umzug in general, we're definitely still using umzug for our migrations!
Just haven't been able to figure out how to use it to manually process ESM migration files - until Sequelize is compatible with v3, perhaps 😉 (And to be fair, it's possible that v2 would work, using Babel, if not for a separate issue with using Babel across our whole codebase while also using Next.js's built-in ESM transpilation strategy??) 🤷