Import broken since v1.0.0/6c210101a283d4fe46bdcf14d33bcdd6317a09ff

[LinqLover]

• Original failure in my project: Bump parse-imports from 0.0.5 to 1.0.0 LinqLover/downstream-repository-mining#37:

      Cannot find module 'parse-imports' from 'src/references.ts'
  
      Require stack:
        src/references.ts
        test/references.test.ts
  
         6 | import _ from "lodash"
         7 | import tqdm from 'ntqdm'
      >  8 | import parseImports from 'parse-imports'
           | ^
         9 | import path from 'path'
        10 |
        11 | import { getCacheDirectory } from './npm-deps'
  
        at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:306:11)
        at Object.<anonymous> (src/references.ts:8:1)
  

• Minimal example to reproduce: https://github.com/LinqLover/parse-imports-example/pull/1

  $ tsc && node index.js
  internal/modules/cjs/loader.js:1085
        throw new ERR_REQUIRE_ESM(filename, parentPath, packageJsonPath);
        ^
  
  Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /home/runner/work/parse-imports-example/parse-imports-example/node_modules/parse-imports/src/index.js
  require() of ES modules is not supported.
  require() of /home/runner/work/parse-imports-example/parse-imports-example/node_modules/parse-imports/src/index.js from /home/runner/work/parse-imports-example/parse-imports-example/index.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 /home/runner/work/parse-imports-example/parse-imports-example/node_modules/parse-imports/src/index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /home/runner/work/parse-imports-example/parse-imports-example/node_modules/parse-imports/package.json.
  
      at Object.Module._extensions..js (internal/modules/cjs/loader.js:1085:13)
      at Module.load (internal/modules/cjs/loader.js:933:32)
      at Function.Module._load (internal/modules/cjs/loader.js:774:14)
      at Module.require (internal/modules/cjs/loader.js:957:19)
      at require (internal/modules/cjs/helpers.js:88:18)
      at Object.<anonymous> (/home/runner/work/parse-imports-example/parse-imports-example/index.js:4:49)
      at Module._compile (internal/modules/cjs/loader.js:1068:30)
      at Object.Module._extensions..js (internal/modules/cjs/loader.js:1097:10)
      at Module.load (internal/modules/cjs/loader.js:933:32)
      at Function.Module._load (internal/modules/cjs/loader.js:774:14) {
    code: 'ERR_REQUIRE_ESM'
  }
  

Note that in both cases, the CI passes when I downgrade to v0.0.5 again. What's happening here? How can I keep using your package after you have upgraded it to ESM?

[TomerAberbach]

Hello again!

So as of v1.0.0 parse-imports only publishes ESM (rather than CJS). If you're not familiar with the difference, then you can read about it on the Node website, but I'll also give you a TL;DR:

• ESM is the newer module format that is now preferred over CJS.
• CJS cannot require ESM, but it can use dynamic import to import ESM asynchronously.
• ESM can import CJS without issue.

Back to your error: what's happening is your tooling is transpiling your TypeScript to CJS and the CJS tries to require parse-imports (which it can't do per my bullet points/the spec) so you get this error.

The solution would be either:

• Easiest: use dynamic import to import parse-imports asynchronously (i.e. const parseImports = (await import('parse-imports')).default)
• Harder: configure ts-node, ts-jest, etc. to transpile to ESM (and add "type": "module" to your package.json). Looks like they have some docs on it:
  • ts-node
  • ts-jest

I hope that helps! Let me know if that solves your issue

[LinqLover]

Thank you very much for the detailed help! :-) However, I've unfortunately not yet managed to get it running.

Until now I have considered the following approaches:

0. Convert my project to an ESM module (if possible, this would actually be my preferred option):

   I do not use ts-node directly but (apparently) through oclif for my CLI. Unfortunately, oclif seems not to support ESM modules yet as per Oclif v2 oclif/oclif#563 (comment).

1. Use dynamic imports on top level (insert const parseImports = (await import('parse-imports')).default on top of the relevant file):

   This gives me the following errors:

   • when running my CLI:

     const parseImports = (await Promise.resolve().then(() => __importStar(require('parse-imports')))).default;
                                 ^^^^^^^
     
     SyntaxError: Unexpected identifier
         at wrapSafe (internal/modules/cjs/loader.js:984:16)
         at Module._compile (internal/modules/cjs/loader.js:1032:27)
         at Module.m._compile (/workspace/downstream-repository-mining/node_modules/ts-node/src/index.ts:1295:23)
         at Module._extensions..js (internal/modules/cjs/loader.js:1097:10)
         at Object.require.extensions.<computed> [as .ts] (/workspace/downstream-repository-mining/node_modules/ts-node/src/index.ts:1298:12)
         at Module.load (internal/modules/cjs/loader.js:933:32)
         at Function.Module._load (internal/modules/cjs/loader.js:774:14)
         at Module.require (internal/modules/cjs/loader.js:957:19)
         at require (internal/modules/cjs/helpers.js:88:18)
         at Object.<anonymous> (/workspace/downstream-repository-mining/src/cli/commands/search.ts:5:1)
     module: @oclif/config@1.17.0
     

     According to SO, this is a complicated way of node to say that an async context is required to use the await.

   • when running my tests using ts-jest:

     "TS1378: Top-level 'await' expressions are only allowed when the 'module' option is set to 'esnext' or 'system', and the 'target' option is set to 'es2017' or higher."
     

     This is a more precise error message! However, I have already set both options to esnext, and a lot of googling did not help me find out what I could be missing else ...

2. Use dynamic imports on method level (insert const parseImports = (await import('parse-imports')).default in the relevant async method):

   This gives me exactly the same errors as described in my initial message, but only when the dynamic import is evaluated.

3. Use synchronous require() instead (const parseImports = require('parse-imports').default):

   This gives me exactly the same errors as described in my initial message, but only when the require() statement is evaluated.

I'm a bit exhausted. :-) Do you maybe have any further idea for me, or do you have a guess which actor is to blame here so I could create an issue in their repository? Thanks in advance!

[TomerAberbach]

0. Yup, makes sense. Tooling is catching up still
1. Yeah, you probably shouldn't try to use top-level dynamic import because top-level await is still in stage 4 (probably should have made that clear before)
2. This is the option that should work. I went and tested that this works with a plain CJS module (not transpiled from TypeScript) and it does work. Turns out this is like three separate issues... A TypeScript specific issue (Add flag to not transpile dynamic import() when module is CommonJS microsoft/TypeScript#43329). Funny enough, looks like your buddies at oclif found a short-term solution for that one. And jest and ts-jest issues that you have to install jest@next and ts-jest@next for. I'm making a PR to your repo with some changes that seem to work
3. This option won't work because CJS cannot require ESM

[LinqLover]

Wow, thank you so very much for your help and explanations in the other thread, Tomer!

Regarding your PR (which I needed to move to LinqLover/downstream-repository-mining#41 to resolve merge conflicts): I actually have no clue why the CI is still not running, but if I try it out locally (./bin/run.js or ./scripts/test-cli.sh), oclif seems to work fine. 👍 The hack for microsoft/TypeScript#43329 is just beautiful ... 😁

Do you see real any danger in using the preview versions for jest? Otherwise, I will merge your PR gladly!

[TomerAberbach]

Wow, thank you so very much for your help and explanations in the other thread, Tomer!

No problem 😄

Regarding your PR (which I needed to move to LinqLover/downstream-repository-mining#41 to resolve merge conflicts): I actually have no clue why the CI is still not running, but if I try it out locally (./bin/run.js or ./scripts/test-cli.sh), oclif seems to work fine. The hack for microsoft/TypeScript#43329 is just beautiful ...

The CI isn't running because you only have your CI set up to run on "push". Although I "pushed" my changes, it wasn't to your repository; it was to my fork (and forks don't run GitHub actions by default I think). I think you have to configure your CI to run on "pull_request" as well (see my ci.yml for this repository).

Do you see real any danger in using the preview versions for jest? Otherwise, I will merge your PR gladly!

I don't think there's any danger in using the preview versions. I assume this isn't a "production" project (yet!) so all it really means is that you may have to modify your configuration again if they decide to change something before releasing the next stable version.

I also think that in general it's good for people (like you) to try out all this ESM stuff because if we don't actively push it, then the tooling isn't gonna move to support it

[LinqLover]

Alright, thank you again so much! 🙏

I also think that in general it's good for people (like you) to try out all this ESM stuff because if we don't actively push it, then the tooling isn't gonna move to support it

True point.

I think you have to configure your CI to run on "pull_request" as well (see my ci.yml for this repository).

Yeah, I wanted to eliminate the duplicate runs ... I had assumed that GitHub Actions would be turned on by default for forks.

[LinqLover]

Hey @TomerAberbach, sorry for disturbing your peace of mind again. :-)

In the last weeks, I have noticed a number of failing builds in my repository which actually turned out to be flaky tests. Whenever the build fails, I see something like this:

$ yarn test --verbose=true --silent=false test/references.test.ts

yarn run v1.22.10
$ cross-env NODE_OPTIONS=--experimental-vm-modules jest --verbose=true --silent=false test/references.test.ts
(node:42760) ExperimentalWarning: VM Modules is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

 RUNS  test/references.test.ts
error Command failed with exit code 1.

(The jest arguments and the specific test file were added for breaking the bug down, but it also occurs when running all tests together). I assume that somehow jest is occasionally dropping the promises generated when a dynamic import is called. This is really a hard flaky test, it only occurs in about 1 of 40 cases (so debugging this was very hard) ... I did git bisect and identified the cause of this issue in your commit LinqLover/downstream-repository-mining@5f3a609. My complete bisect script for every revision looks like this:

yarn install && yarn test --clearCache && yarn test test/references.test.ts -i --verbose=true --silent=false && (set -e ; for run in {1..100}; do echo "ATTEMPT $run" && yarn test --verbose=true --silent=false test/references.test.ts || exit 1; done && echo good)

I already was able to reduce the failure ratio from ~1/2 to ~1/40 by reducing the number of dynamic imports (see LinqLover/downstream-repository-mining@472d90f), but it is still awful to work with unreliable tests.

I also have opened jestjs/jest#11601, but I'm not sure if they will be able to help me.
Just on the off-chance, do you hypothetically have any other idea how to get rid of this problem? I know I cannot expect this, otherwise please just treat this as FYIO and self-reflectional post. :-)

[TomerAberbach]

Hey! Sorry for the late reply; I was moving!

Yeah, I'm not really sure what's going on there. It seems like it could be a Jest issue as you suggested.