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]: ts-jest silently hangs up with certain moduleResolution options #4207

Closed
BenceSzalai opened this issue Sep 13, 2023 · 13 comments · Fixed by #4226
Closed

[Bug]: ts-jest silently hangs up with certain moduleResolution options #4207

BenceSzalai opened this issue Sep 13, 2023 · 13 comments · Fixed by #4226

Comments

@BenceSzalai
Copy link

BenceSzalai commented Sep 13, 2023

Version

29.1.1

Steps to reproduce

  1. Open minimal reproduction https://stackblitz.com/edit/sbnc-tsnode-hang-issue?file=package.json
  2. Wait for npm install
  3. Run npm run test

Expected behavior

The test should either run or give some error.

Actual behavior

The test hangs up forever.

Debug log

You can find the debug log in the reproduction: ts-jest.log

Additional context

Please check out jest.config.js. It only has two meaningful config items: extensionsToTreatAsEsm and transform.isolatedModules. Comment out any of them, and the test run fails properly with an error message. Leave them both in, and the test hangs up forever.

The issue is related to moduleResolution in tsconfig.json. Set it to node10 or classic and there is no hang, set it to node16, nodenext or bundler and the issue comes up.

Please note, that it is very minimal, I've removed everything not related to the issue. So ignore if this configuration does not make sense, I've narrowed it down from a much more complex real issue I had. The main point here is that ts-jest hangs without any indication of an error, which is very painful to debug.

The reproduction is on node v16, but the same happens on Node v20 as well.

Environment

System:
    OS: Linux 5.0 undefined
    CPU: (4) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
  Binaries:
    Node: 16.20.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.4.2 - /usr/local/bin/npm
    pnpm: 8.6.10 - /usr/local/bin/pnpm
  npmPackages:
    jest: ^29.0.0 => 29.7.0
@mscharley
Copy link

I'm pretty sure this is what I'm running into as well, for a slightly less minimal example if it's useful then have a look at mscharley/dot#63

Seems to only be an issue with TS ~5.2, 5.1 is unaffected. It's similarly happening across Node 16, 18, and 20 for me as well.

I'd also be interested to know why it seems like ts-jest is messing with moduleResolution and especially module settings - it can trigger very different behaviour in projects if it gets toggled between different values. In my project I have both module and moduleResolution set explicitly to node16 but I managed to get the same error out of Jest:

  ● Test suite failed to run

    error TS5110: Option 'module' must be set to 'Node16' when option 'moduleResolution' is set to 'Node16'.

@BenceSzalai
Copy link
Author

But are you getting a hang as well? I would have been happy about an error message tbh. Hang is way harder to deal with, and has some dire implications in a CI environment eating away server time...

@mscharley
Copy link

mscharley commented Sep 14, 2023

Yep, I'm getting the hang with no changes other than the typescript package version update, eg: https://github.com/mscharley/ioc-deco/actions/runs/5994385300/job/16255936781?pr=63

I got the error message by disabling NODE_OPTIONS=--experimental-vm-modules, similar to how you did modifying some of Jest's options.

@ValentinGurkov
Copy link

ValentinGurkov commented Sep 18, 2023

I don't have a minimal reproduction repo but I'm running into the same thing with typescript 5.2.2.
Jest hangs.
If I remove NODE_OPTIONS=--experimental-vm-modules I get:

error TS5110: Option 'module' must be set to 'NodeNext' when option 'moduleResolution' is set to 'NodeNext'.

Even though I already have:

"module": "NodeNext",
"moduleResolution": "NodeNext"

in my tsconfig.json.

Tried with Node 18 and 20.

@mscharley
Copy link

mscharley commented Sep 19, 2023

Borrowing from #4198 I tried using a special tsconfig for just jest:

{
	"extends": "./tsconfig.json",
	"compilerOptions": {
		"module": "ESNext",
		"moduleResolution": "Node10"
	}
}

This is working for me, even for node imports which wouldn't normally work. This is very much a workaround though. This feels like a combination of two issues:

  1. Per the comments in Support Node16/NodeNext value for moduleResolution #4198 ts-jest is modifying the module compilation option used to compile typescript with when it is in ESM mode but isn't modifying moduleResolution as well which is now an error in TS >= 5.2 if you have both explicitly specified in your tsconfig.
  2. This compilation error isn't being handled properly somewhere inside either Jest or ts-jest and the compilation error is leading to a hang condition (this issue).

@noahm
Copy link

noahm commented Oct 13, 2023

@mscharley excellent workaround, thanks so much for sharing!

@SimenB
Copy link
Contributor

SimenB commented Nov 26, 2023

I dug around a bit in the repro provided in the link above.

In that case, we hit this throw statement:

throw new Error(interpolate(Errors.CannotProcessFile, { file: fileName }))

Called from here:

const processWithTsResult = this.processWithTs(sourceText, sourcePath, transformOptions)

The problem is that it throws within a new Promise which is not properly propagated.

process.on('unhandledRejection', err => {
    console.error('unhandled rejection', err);
})

new Promise(async (resolve, reject) => {
    throw new Error('Go boom');
}).then(() => {
    console.log('resolved');
}).catch(err => {
    console.error('caught', err);
});

Running this will trigger the unhandled rejection instead of bubbling up the error.

Wrapping in try-catch and calling reject would fix it, but I don't understand why the new Promise is there at all - removing that wrapper and just using await within processAsync normally would also resolve the issue.

This diff correctly bubbles up the error:

diff --git i/src/legacy/ts-jest-transformer.ts w/src/legacy/ts-jest-transformer.ts
index b4dbdc2a2..b0c3b5366 100644
--- i/src/legacy/ts-jest-transformer.ts
+++ w/src/legacy/ts-jest-transformer.ts
@@ -189,7 +189,6 @@ export class TsJestTransformer implements SyncTransformer {
   ): Promise<TransformedSource> {
     this._logger.debug({ fileName: sourcePath, transformOptions }, 'processing', sourcePath)
 
-    return new Promise(async (resolve, reject) => {
       const configs = this._configsFor(transformOptions)
       const shouldStringifyContent = configs.shouldStringifyContent(sourcePath)
       const babelJest = shouldStringifyContent ? undefined : configs.babelJestTransformer
@@ -199,7 +198,7 @@ export class TsJestTransformer implements SyncTransformer {
         code: processWithTsResult.code,
       }
       if (processWithTsResult.diagnostics?.length) {
-        reject(configs.createTsError(processWithTsResult.diagnostics))
+        throw configs.createTsError(processWithTsResult.diagnostics)
       }
       if (babelJest) {
         this._logger.debug({ fileName: sourcePath }, 'calling babel-jest processor')
@@ -212,8 +211,7 @@ export class TsJestTransformer implements SyncTransformer {
       }
       result = this.runTsJestHook(sourcePath, sourceText, transformOptions, result)
 
-      resolve(result)
-    })
+      return result
   }
 
   private processWithTs(
image

(Also, I don't know why async-await is transpiled)


That said, Jest should have some sort of unhandled rejection listener somewhere that picks this up - unsure why that doesn't happen...

@SimenB
Copy link
Contributor

SimenB commented Nov 26, 2023

Opened #4226 fixing the error being gobbled up (which is what this issue is about - not what error is being thrown)

@darkbasic
Copy link

@SimenB regarding the error itself did I misconfigure something or is it yet another issue?

@SimenB
Copy link
Contributor

SimenB commented Nov 26, 2023

Dunno, I haven't used those options. Error message says to ensure outDir

@darkbasic
Copy link

Error message says to ensure outDir

Which unfortunately makes no sense either. For esm javascript they suggest to use an empty transform but obviously I can't do that with Typescript.

@darkbasic
Copy link

darkbasic commented Nov 27, 2023

I managed to get it working: https://github.com/darkbasic/jest-esm-repro
Unfortunately we still have problems with errors not bubbling up, in fact the culprit was "noEmitOnError": true. Removing it fixed jest but still no clue which errors are supposed to happen because yarn compile works fine with the same tsconfig.

@darkbasic
Copy link

Even worse I've just noticed that snapshots don't work at all: jestjs/jest#14723

akheron added a commit to espoon-voltti/evaka that referenced this issue Nov 29, 2023
There's a bug in ts-jest that makes jest hang if tsconfig.json has a
certain combination of module and moduleResolution values. Fix this by
providing a differend tsconfig for ts-node.

See kulshekhar/ts-jest#4198 and
kulshekhar/ts-jest#4207.
akheron added a commit to espoon-voltti/evaka that referenced this issue Nov 29, 2023
There's a bug in ts-jest that makes jest hang if tsconfig.json has a
certain combination of module and moduleResolution values. Fix this by
providing a differend tsconfig for ts-node.

See kulshekhar/ts-jest#4198 and
kulshekhar/ts-jest#4207.
Vylpes pushed a commit to Vylpes/Droplet that referenced this issue Apr 10, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [ts-jest](https://kulshekhar.github.io/ts-jest) ([source](https://github.com/kulshekhar/ts-jest)) | devDependencies | patch | [`29.1.1` -> `29.1.2`](https://renovatebot.com/diffs/npm/ts-jest/29.1.1/29.1.2) |

---

### Release Notes

<details>
<summary>kulshekhar/ts-jest (ts-jest)</summary>

### [`v29.1.2`](https://github.com/kulshekhar/ts-jest/blob/HEAD/CHANGELOG.md#2912-2024-01-22)

[Compare Source](kulshekhar/ts-jest@v29.1.1...v29.1.2)

##### Bug Fixes

-   calculated cache key based on `supportsStaticESM` ([a5d6f2d](kulshekhar/ts-jest@a5d6f2d))
-   correct error handling in `processAsync` ([e7be4bf](kulshekhar/ts-jest@e7be4bf)), closes [#&#8203;4207](kulshekhar/ts-jest#4207)
-   use `Config.ProjectConfig` ([918312b](kulshekhar/ts-jest@918312b)), closes [#&#8203;4028](kulshekhar/ts-jest#4028)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4wLjAiLCJ1cGRhdGVkSW5WZXIiOiIzNy4wLjAiLCJ0YXJnZXRCcmFuY2giOiJkZXZlbG9wIn0=-->

Reviewed-on: https://gitea.vylpes.xyz/RabbitLabs/Droplet/pulls/264
Reviewed-by: Vylpes <[email protected]>
Co-authored-by: Renovate Bot <[email protected]>
Co-committed-by: Renovate Bot <[email protected]>
Vylpes pushed a commit to Vylpes/vylbot-app that referenced this issue Apr 10, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [ts-jest](https://kulshekhar.github.io/ts-jest) ([source](https://github.com/kulshekhar/ts-jest)) | dependencies | patch | [`29.1.1` -> `29.1.2`](https://renovatebot.com/diffs/npm/ts-jest/29.1.1/29.1.2) |

---

### Release Notes

<details>
<summary>kulshekhar/ts-jest (ts-jest)</summary>

### [`v29.1.2`](https://github.com/kulshekhar/ts-jest/blob/HEAD/CHANGELOG.md#2912-2024-01-22)

[Compare Source](kulshekhar/ts-jest@v29.1.1...v29.1.2)

##### Bug Fixes

-   calculated cache key based on `supportsStaticESM` ([a5d6f2d](kulshekhar/ts-jest@a5d6f2d))
-   correct error handling in `processAsync` ([e7be4bf](kulshekhar/ts-jest@e7be4bf)), closes [#&#8203;4207](kulshekhar/ts-jest#4207)
-   use `Config.ProjectConfig` ([918312b](kulshekhar/ts-jest@918312b)), closes [#&#8203;4028](kulshekhar/ts-jest#4028)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4wLjAiLCJ1cGRhdGVkSW5WZXIiOiIzNy4wLjAiLCJ0YXJnZXRCcmFuY2giOiJkZXZlbG9wIn0=-->

Reviewed-on: https://gitea.vylpes.xyz/RabbitLabs/vylbot-app/pulls/400
Co-authored-by: Renovate Bot <[email protected]>
Co-committed-by: Renovate Bot <[email protected]>
Vylpes pushed a commit to Vylpes/random-bunny that referenced this issue Apr 10, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [ts-jest](https://kulshekhar.github.io/ts-jest) ([source](https://github.com/kulshekhar/ts-jest)) | devDependencies | patch | [`29.1.1` -> `29.1.2`](https://renovatebot.com/diffs/npm/ts-jest/29.1.1/29.1.2) |

---

### Release Notes

<details>
<summary>kulshekhar/ts-jest (ts-jest)</summary>

### [`v29.1.2`](https://github.com/kulshekhar/ts-jest/blob/HEAD/CHANGELOG.md#2912-2024-01-22)

[Compare Source](kulshekhar/ts-jest@v29.1.1...v29.1.2)

##### Bug Fixes

-   calculated cache key based on `supportsStaticESM` ([a5d6f2d](kulshekhar/ts-jest@a5d6f2d))
-   correct error handling in `processAsync` ([e7be4bf](kulshekhar/ts-jest@e7be4bf)), closes [#&#8203;4207](kulshekhar/ts-jest#4207)
-   use `Config.ProjectConfig` ([918312b](kulshekhar/ts-jest@918312b)), closes [#&#8203;4028](kulshekhar/ts-jest#4028)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4wLjAiLCJ1cGRhdGVkSW5WZXIiOiIzNy4wLjAiLCJ0YXJnZXRCcmFuY2giOiJkZXZlbG9wIn0=-->

Reviewed-on: https://gitea.vylpes.xyz/RabbitLabs/random-bunny/pulls/143
Reviewed-by: Vylpes <[email protected]>
Co-authored-by: Renovate Bot <[email protected]>
Co-committed-by: Renovate Bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants