Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ES Modules broken on npm@7 #4295

Closed
richardsimko opened this issue Feb 15, 2021 · 5 comments
Closed

ES Modules broken on npm@7 #4295

richardsimko opened this issue Feb 15, 2021 · 5 comments

Comments

@richardsimko
Copy link
Contributor

Environment

Knex version: 0.21.17
Database + version: N/A
OS: N/A

Bug

  1. Explain what kind of behaviour you are getting and how you think it should do

When setting type: 'module' in package.json knex fails to use ESM to load the migration files on npm >= 7.0.0. This works on lower versions. The reason being that npm has removed the environment variable which knex uses to determine if it should use ESM or CommonJS (

const isTypeModule = process.env.npm_package_type === 'module';
). See this NPM RFC for more details https://github.com/npm/rfcs/blob/a044b355af60310b05ad1995fdc1cf672243a4e0/implemented/0021-reduce-lifecycle-script-environment.md. If the file is named knexfile.mjs it works as expected since the filename check triggers the usage of ESM.

  1. Error message
Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /project-root/config/knexfile.js
require() of ES modules is not supported.
require() of /project-root/node_modules/knex/lib/util/import-file.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename knexfile.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /project-root/package.json.

    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1080:13)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at importFile (/project-root/node_modules/knex/lib/util/import-file.js:13:7)
    at openKnexfile (/project-root/node_modules/knex/bin/cli.js:26:22)
    at initKnex (/project-root/node_modules/knex/bin/cli.js:47:13)
    at Command.<anonymous> (/project-root/node_modules/knex/bin/cli.js:170:32)
    at Command.listener [as _actionHandler] (/project-root/node_modules/commander/index.js:426:31)
  1. Reduced test code, for example in https://npm.runkit.com/knex or if it needs real
    database connection to MySQL or PostgreSQL, then single file example which initializes
    needed data and demonstrates the problem.

All that's needed is a project with type: 'module' in the package.json and a knexfile ending in .js.

npm's recommended migration route is to use the variable npm_package_json which points to the package.json, load the JSON and then read values from there.

@kibertoad
Copy link
Collaborator

@richardsimko Uh-oh, thank you for pointing out the issue! Would you consider sending a PR with a fix?

@richardsimko
Copy link
Contributor Author

Sure, I can probably put something together. Just in case anyone else stumbles on this before a fix is released, a workaround is to change the knexfile to end with .mjs.

richardsimko pushed a commit to richardsimko/knex that referenced this issue Feb 15, 2021
@richardsimko
Copy link
Contributor Author

@kibertoad I opened #4296

@kibertoad
Copy link
Collaborator

Released in 0.95.0-next2

richardsimko pushed a commit to richardsimko/knex that referenced this issue Feb 22, 2021
@kibertoad
Copy link
Collaborator

Released in 0.21.18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants