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

[Bug]: incorrect behaviour of requireOrImportModule from jest-util #14013

Closed
koshic opened this issue Mar 16, 2023 · 8 comments
Closed

[Bug]: incorrect behaviour of requireOrImportModule from jest-util #14013

koshic opened this issue Mar 16, 2023 · 8 comments

Comments

@koshic
Copy link

koshic commented Mar 16, 2023

Version

29.5.0

Steps to reproduce

Currently I can't provide repo with full demo, sorry.

The issue happens in ESM setup, when external files passed to Jest are must be loaded with '--experimental-loader=SOME_LOADER'. In my case, it's a custom loader to transform .ts files, and it works fine, 100% according to Node specs.

I can execute this file manually, like 'node --loader MY_LOADER ./custom-transformer.ts", but when I pass file path to Jest in TS-written config, it fails: '
image

Why? The root cause is requireOrImportModule logic:

  try {
    const requiredModule = require(filePath);
    if (!applyInteropRequireDefault) {
      return requiredModule;
    }
    return interopRequireDefault(requiredModule).default;
  } catch (error: any) {
    if (error.code === 'ERR_REQUIRE_ESM') {

Jest expects that require('custom-transformer.ts') will fail with ERR_REQUIRE_ESM, if custom-transformer.ts is ESM module. This assumption is incorrect - Node guarantee only that .js (with type: module in package.json) / .mjs file can't be required this way, internal CJS loader will throws ERR_REQUIRE_ESM.

For any other extensions it's not true - because CJS & ESM are different worlds. Node not even try to call ESM loader on require() call, ESM loader is only activated on imports.

As a result, we have a bit strange picture: you can't use pure ESM loader with Jest. You MUST intercept require() calls (via Module._extensions, as an example) and return ERR_REQUIRE_ESM for files... already covered by your loader.

Sure, it's not a big deal to add more or less ugly interception code somewhere (not in jest config file, hehe - because workers have their own Module instance, and it's too late to make a trick with NODE_MODULES env variable). But it would be nice have fix in Jest. May be just by trying import() first (yeah, naive solution), or adding some flag / global switch 'please handle all modules as ESM, thx' (as it implemented for --experimental-vm-modules), etc.

PS I'm not sure which kind of issue I taking about - bug or feature request, because current implementation works fine for the most cased, and it's very few projects used ESM-only modules everywhere (and don't maintain compatibility with CJS at all).

Expected behavior

ESM module imported without issues.

Actual behavior

Jest fails.

Additional context

No response

Environment

System:
    OS: macOS 13.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
  Binaries:
    Node: 19.8.1 - ~/.nvm/versions/node/v19.8.1/bin/node
    Yarn: 3.3.1 - ~/.nvm/versions/node/v19.8.1/bin/yarn
    npm: 9.5.1 - ~/.nvm/versions/node/v19.8.1/bin/npm
  npmPackages:
    jest: workspace:^ => 29.5.0
@mrazauskas
Copy link
Contributor

Is #13521 roughly related to what you are looking for?

@koshic
Copy link
Author

koshic commented Mar 16, 2023

@mrazauskas very roughly - the same intention, but in different direction: I need to bypass incorrect CJS / ESM detection on loading certain files, whereas that PR (a bit naive and incorrect, but it's ok for a draft) targeted to use existing loader as transformer.

So, even if #13521 will be landed, requireOrImportModule issue still exists - for other files which can be specified in config to customize Jest behaviour.

@mrazauskas
Copy link
Contributor

By the way, if I get it right requireOrImportModule is just a hack. It was added to the code base as a temporary utility to help migrating to ESM. See #11167 (comment)

That’s painful topic, because the migration got stuck. Reasons are the same why Node still has --experimental-vm-modules flag. These are some long standing bugs in V8.

@koshic
Copy link
Author

koshic commented Mar 19, 2023

@mrazauskas we have to make clear distinguish between 'Jest can work in ESM environment itself' (what is that issue about) and 'Jest can run tests in ESM mode via Nodes's VM' (which is a long and sad story, right).

Also, there are some projects trying to solve VM issue (by eliminating...) - I mean https://github.com/nicolo-ribaudo/jest-light-runner.

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Apr 18, 2023
@github-actions
Copy link

This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 18, 2023
@github-actions
Copy link

This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants