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

Cannot import ESM modules when using TypeScript #4447

Closed
vvo opened this issue Apr 19, 2021 · 6 comments
Closed

Cannot import ESM modules when using TypeScript #4447

vvo opened this issue Apr 19, 2021 · 6 comments

Comments

@vvo
Copy link
Contributor

vvo commented Apr 19, 2021

Environment

Knex version: 0.95.4
Database + version: sqlite3 5.0.2
OS: macOS Big Sur 11.1

@lorefnon (since related to TypeScript)

Hi there, I spent quite a bunch of time on this without finding the proper way to resolve it.

Bug

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

When trying to import randomInt from "random-int" from a TypeScript migration file we get an error.

  1. Error message
Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /Users/[redacted]/knex-typescript-migrations-esm/node_modules/random-int/index.js
require() of ES modules is not supported.
require() of /Users/[redacted]/knex-typescript-migrations-esm/node_modules/random-int/index.js from /Users/[redacted]/knex-typescript-migrations-esm/migrations/20210419131555_test.ts 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 index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /Users/[redacted]/knex-typescript-migrations-esm/node_modules/random-int/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 Object.<anonymous> (/Users/[redacted]/knex-typescript-migrations-esm/migrations/20210419131555_test.ts:2:1)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Module.m._compile (/Users/[redacted]/knex-typescript-migrations-esm/node_modules/ts-node/src/index.ts:1056:23)
    at Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Object.require.extensions.<computed> [as .ts] (/Users/[redacted]/knex-typescript-migrations-esm/node_modules/ts-node/src/index.ts:1059:12)

Running with --esm sort of works but breaks TypeScript:

export async function up(knex: Knex): Promise<void> {
                             ^

SyntaxError: Invalid or unexpected token
    at Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
  1. Reduced test code: https://github.com/vvo/knex-typescript-migrations-esm

Thanks!

@cayter
Copy link

cayter commented May 8, 2021

I looked into the source code and found:

  • the migrator is relying on this to decide if it should use import() to load the .ts file.
  • however, the function it's relying on only works with:
    • .mjs file extension
    • running npm_package_json=package.json knex migrate:latest with "type": "module" configured in the package.json
    • running npm_package_type=module knex migrate:latest

I tested by setting npm_package_type=module approach and it worked.

If you're interested in fixing it and submit a PR, it's as easy as fixing https://github.com/knex/knex/blob/master/lib/migrations/util/is-module-type.js to also check filepath that ends with .ts:

const { readFile } = require('./fs');

module.exports = async function isModuleType(filepath) {
  ...

  return process.env.npm_package_type === 'module' || filepath.endsWith('.mjs') || filepath.endsWith('.ts');
};

However, I think the code above isn't solid enough. By right, it should check if the filepath ends with any value configured in MigratorConfig.extensions which is the single source of truth for our migrator configuration.

@kibertoad
Copy link
Collaborator

@cayter Thank you for looking into it! Would you consider sending a PR for this?

@azban
Copy link

azban commented Jun 2, 2021

not sure this is totally relevant, but seems like https://github.com/cfware/get-package-type#readme may be a robust alternative to using npm_package_json.. especially given that the yarn 2 core dev(s?) seem opposed to adding that variable. as a result, i'm guessing this is currently broken with yarn 2.

see:

@lehni
Copy link
Contributor

lehni commented Mar 18, 2022

I've created a PR for this now, I hope this solves it for everybody: #5071

@lehni
Copy link
Contributor

lehni commented Mar 19, 2022

With #5071 merged (81b7edd), I believe this issue can be closed now, cc @kibertoad

@kibertoad
Copy link
Collaborator

Released in 1.0.5

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

No branches or pull requests

6 participants