Add esbuild resolver for `jest.config.ts` #12041

jensmeindertsma · 2021-11-04T19:29:03Z

Summary

I was using Jest with a jest.config.ts config file written in TypeScript and I found it annoying that I had to make changes to my tsconfig.json file in order for it to work. This pull request adds a fallback option to jest-config where if ts-node isn't installed it will try to use esbuild instead. esbuild is faster, doesn't check types ( I found this annoying, Jest doesn't check the types of my tests, so why should it for my config? Let me check my own types please :) ), and supports importing other files in your config file ( not sure whether ts-node supported this ).

Test plan

I made the change, then ran these commands to verify things work:

npm run jest packages/jest-config // these tests pass
cd packages/jest-config
npm pack // gives a .tar.gz file
cd ~/Code ( whatever works for you )
gh repo clone jensmeindertsma/package-template
npm install --save-dev esbuild <FILE_URL_FROM_NPM_PACK>
npm uninstall ts-node

// Wipe lockfile so `ts-node` is properly removed
rm -rf node_modules package-lock.json
npm install

npm run test

The tests in my demo repository are passing, which means this change works! It successfully loaded the TS config file without ts-node installed.

Oh yeah, this closes #11989 and #11453, although the latter one probably requires some docs updates.

facebook-github-bot · 2021-11-04T19:29:07Z

Hi @jensmeindertsma!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

facebook-github-bot · 2021-11-04T20:29:12Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

facebook-github-bot · 2021-11-04T20:51:34Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

jensmeindertsma · 2021-11-05T04:58:13Z

Just need to fix some dependency conflicts, will get to that within a few hours.

G-Rath

I found this annoying, Jest doesn't check the types of my tests, so why should it for my config? Let me check my own types please :)

Because it's not Jest doing that, it's TypeScript, and it's easier for the user to disable this behaviour than it is to try and re-enable it after we've hard-coded disabling it. This also matches the general default for the rest of the TS ecosystem (e.g. ts-jest & typescript both check types by default)

supports importing other files in your config file ( not sure whether ts-node supported this ).

Yes it does.

I think there's value in supporting options like this, especially when there's not a whole lot of extra code required + esbuild is pretty popular these days :)

G-Rath · 2021-11-05T05:04:27Z

packages/jest-config/src/readConfigFileAndSetRootDir.ts

+
+      await esbuild({
+        // By bundling we add support for importing stuff in the config file.
+        bundle: true,


I don't think it's a huge risk, but fwiw arguably there's a security angle here where if someone were to import a dependency which itself imported a lot of files, the resulting file on disk would be huge.

Yeah, I don't think this would be a problem. I added this option only because I was told there will be users who import other files in their config file, then this is a necessity.

packages/jest-config/src/readConfigFileAndSetRootDir.ts

codecov-commenter · 2021-11-05T07:17:44Z

Codecov Report

Merging #12041 (6ad8d3b) into main (95f4969) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main   #12041      +/-   ##
==========================================
- Coverage   68.77%   68.75%   -0.03%     
==========================================
  Files         324      324              
  Lines       16670    16675       +5     
  Branches     4814     4815       +1     
==========================================
  Hits        11465    11465              
- Misses       5172     5177       +5     
  Partials       33       33

Impacted Files	Coverage Δ
...ges/jest-config/src/readConfigFileAndSetRootDir.ts	`2.56% <0.00%> (-0.38%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95f4969...6ad8d3b. Read the comment docs.

G-Rath

What use cases are there for dynamic importing in config files?

The immediate one that springs to mind is importing an ESM based package, since that requires a promise. Outside of that, I imagine there are probably folks (and companies) out there that are doing more logic than we'd initially expect in their configs 😅

I don't think this is necessarily a blocker to not have, but is something that'd be nice in the long-run, especially since it's already supported by ts-node.

G-Rath · 2021-11-06T20:54:18Z

packages/jest-config/src/readConfigFileAndSetRootDir.ts

+      try {
+        require('esbuild');
+      } catch (error: any) {
+        if (error.code === 'MODULE_NOT_FOUND') {
+          throw new Error(
+            `Jest: 'ts-node' or 'esbuild' is required for the TypeScript configuration files. Make sure it is installed\nError: ${e.message}`,
+          );
+        }
+
+        throw error;
+      }
+
+      registerer = require('esbuild-register').register();


The requiring of esbuild-register should be done in this try/catch, and the error message should be updated to say esbuild-register instead of esbuild.

I don't think we need to be checking that esbuild itself is installed as it's a peer dependency of esbuild-register so that package should be handling checking if it's available (+ package managers will say that its needed due to being listed as a peer dependency)

G-Rath · 2021-11-06T21:00:37Z

packages/jest-config/src/readConfigFileAndSetRootDir.ts

+  if (tsNode) {
+    registerer.enabled(false);
+  }


The registerer needs to be disabled before this function returns because otherwise it will continue to run on unrelated files which will mess with Jest's transformation abilities & expectations, and cause a bunch of weird bugs (e.g. Jest can end up getting js versions of ts files, which'll not get mapped back to their sources when showing failure messages & stack traces).

Sadly it looks like esbuild-register doesn't support being enabled and disabled, so that will a block - however it shouldn't be hard to implement and if it's done with the same API as ts-node we should be able to leave these enabled calls as is.

Should I open an issue on esbuild-register?

G-Rath · 2021-11-06T21:01:25Z

packages/jest-config/package.json

@@ -14,9 +14,13 @@
    "./package.json": "./package.json"
  },
  "peerDependencies": {
+    "esbuild": ">=0.13.12",


This should be replaced with esbuild-register, since that is what Jest actually needs to do the work.

You're right, however esbuild-register specifies esbuild as a peerDependency so we'll still need the user to install esbuild

jensmeindertsma · 2021-11-08T06:37:35Z

~~Blocked by egoist/esbuild-register#41~~

jensmeindertsma · 2021-11-08T16:06:55Z

Okay, this is now ready! esbuild-register is now disabled when we're done reading the TS file.

To use this, the user must install esbuild and esbuild-register in their projects.

Zexuz · 2022-01-20T11:49:14Z

What's the status on this?

Zexuz · 2022-04-28T08:23:02Z

@G-Rath Any estimate on a timeline?

G-Rath · 2022-05-03T23:16:01Z

@Zexuz I'm just a contributor to Jest, not a member so I can't merge this (I can't even approve it because I don't have any access to the repo, due to GitHub's new rules) - as it stands the PR looks almost good to go, it just needs to be rebased (I think because the upstream fork has been deleted, GH isn't saying it needs rebasing but it does).

@SimenB is the one who manages Jest so is the person whose approval you really need to get this merged.

@jensmeindertsma are you still interested in landing this? I might be able to pick it up if @SimenB is happy with the approach, but I don't currently use esbuild so if he's not I can't promise I'll be willing to carry this through vs working on other projects right now.

(a possible alternative that I would be interested in discussing is making some ts-registererer package that enscapulsates this logic and provides a common api so packages like jest can just have that as a dependency instead, as I've implemented this across a few projects now and there's no reason why they too couldn't be using alternatives like esbuild)

SimenB · 2022-11-07T17:29:45Z

My current thinking is #13143 (comment) - we shouldn't change our default, but we should allow people to define what they want to use as a docblock comment. Happy to take a PR which does that 👍

github-actions · 2023-11-20T00:08:18Z

This pull request 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.

Add esbuild resolver for jest.config.ts

2b1a7ff

Update CHANGELOG.md

8dbe6fd

facebook-github-bot added the cla signed label Nov 4, 2021

G-Rath reviewed Nov 5, 2021

View reviewed changes

jensmeindertsma added 3 commits November 5, 2021 07:34

Fix lockfile

93c0a87

Use OS temp directory

da0c199

Use esbuild-register

2d8deb5

G-Rath reviewed Nov 6, 2021

View reviewed changes

jensmeindertsma mentioned this pull request Nov 7, 2021

Allow programmatic enabling and disabling of esbuild-register egoist/esbuild-register#40

Closed

Update code

8e00bc0

jensmeindertsma mentioned this pull request Nov 8, 2021

feat: Allow programmatic enabling / disabling of esbuild-register egoist/esbuild-register#41

Merged

jensmeindertsma added 2 commits November 8, 2021 17:03

Update to latest version of esbuild-register

0bcaacd

Fix lockfile

6ad8d3b

MasterOdin mentioned this pull request Sep 16, 2022

[Feature]: Allow using tsx instead of ts-node for loading TS config files #13143

Open

MasterOdin mentioned this pull request Jan 7, 2023

feat(jest-config): Support using esbuild-register for loading TS configs #13742

Closed

jensmeindertsma closed this Oct 20, 2023

github-actions bot locked as resolved and limited conversation to collaborators Nov 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add esbuild resolver for `jest.config.ts` #12041

Add esbuild resolver for `jest.config.ts` #12041

jensmeindertsma commented Nov 4, 2021 •

edited

Loading

facebook-github-bot commented Nov 4, 2021

facebook-github-bot commented Nov 4, 2021

facebook-github-bot commented Nov 4, 2021

jensmeindertsma commented Nov 5, 2021

G-Rath left a comment •

edited

Loading

G-Rath Nov 5, 2021 •

edited

Loading

jensmeindertsma Nov 5, 2021

codecov-commenter commented Nov 5, 2021 •

edited

Loading

G-Rath left a comment

G-Rath Nov 6, 2021

G-Rath Nov 6, 2021 •

edited

Loading

jensmeindertsma Nov 7, 2021

G-Rath Nov 6, 2021

jensmeindertsma Nov 7, 2021

jensmeindertsma commented Nov 8, 2021 •

edited

Loading

jensmeindertsma commented Nov 8, 2021

Zexuz commented Jan 20, 2022

Zexuz commented Apr 28, 2022

G-Rath commented May 3, 2022

SimenB commented Nov 7, 2022

github-actions bot commented Nov 20, 2023

Add esbuild resolver for jest.config.ts #12041

Add esbuild resolver for jest.config.ts #12041

Conversation

jensmeindertsma commented Nov 4, 2021 • edited Loading

Summary

Test plan

facebook-github-bot commented Nov 4, 2021

Action Required

Process

facebook-github-bot commented Nov 4, 2021

facebook-github-bot commented Nov 4, 2021

jensmeindertsma commented Nov 5, 2021

G-Rath left a comment • edited Loading

Choose a reason for hiding this comment

G-Rath Nov 5, 2021 • edited Loading

Choose a reason for hiding this comment

jensmeindertsma Nov 5, 2021

Choose a reason for hiding this comment

codecov-commenter commented Nov 5, 2021 • edited Loading

Codecov Report

G-Rath left a comment

Choose a reason for hiding this comment

G-Rath Nov 6, 2021

Choose a reason for hiding this comment

G-Rath Nov 6, 2021 • edited Loading

Choose a reason for hiding this comment

jensmeindertsma Nov 7, 2021

Choose a reason for hiding this comment

G-Rath Nov 6, 2021

Choose a reason for hiding this comment

jensmeindertsma Nov 7, 2021

Choose a reason for hiding this comment

jensmeindertsma commented Nov 8, 2021 • edited Loading

jensmeindertsma commented Nov 8, 2021

Zexuz commented Jan 20, 2022

Zexuz commented Apr 28, 2022

G-Rath commented May 3, 2022

SimenB commented Nov 7, 2022

github-actions bot commented Nov 20, 2023

Add esbuild resolver for `jest.config.ts` #12041

Add esbuild resolver for `jest.config.ts` #12041

jensmeindertsma commented Nov 4, 2021 •

edited

Loading

G-Rath left a comment •

edited

Loading

G-Rath Nov 5, 2021 •

edited

Loading

codecov-commenter commented Nov 5, 2021 •

edited

Loading

G-Rath Nov 6, 2021 •

edited

Loading

jensmeindertsma commented Nov 8, 2021 •

edited

Loading