feat: bundle Jest's modules with webpack
Summary
Bundling our modules allows us to improve our startup time by avoiding a bunch of extra I/O. Optimized code also should run faster (in theory), but might be worth it to drop that for readable code in node_modules?
Anyways, there are (at least) 2 outstanding issues.
jest-workerneeds to pass the path to a file tochild_processso it can be executed. I've been unable to make Webpack split out a separate file on disk, then return an absolute path to it. Might need to do very custom stuff just for jest-worker, not sure (if possible, keep just plain babel run for this module for now, if everything else works. Hard to tell since withoutjest-worker, we cannot run anything at all)- There are some places we reach into
buildon purpose (such as forjest-circus/runner). We need to do something Clever :tm: here. (I haven't bothered thinking too much about as I've swapped between fightingrollupandwebpackthe last few days).
Any and all help greatly appreciated!
Problem areas: https://github.com/facebook/jest/blob/29534331e2d3d3f70cf153d7e53569d9dd80ecfe/packages/jest-worker/src/workers/NodeThreadsWorker.ts#L61 & https://github.com/facebook/jest/blob/29534331e2d3d3f70cf153d7e53569d9dd80ecfe/packages/jest-worker/src/workers/ChildProcessWorker.ts#L86
Note that worker_threads can receive source code as string, so I wondered if just inlining could work (albeit still a separate bundle), but child_process does not, so doesn't help
Another alternative is to inline as bundled string, then write to disk during load. Feels wrong tho
Speaking of rollup, I cannot make it understand require, and it also barfs on import thing = require('thing') even though babel is supposed to compile it away. Anyways, let's try webpack first.
Test plan
Green CI (at some point)
Speaking of rollup, I cannot make it understand require, and it also barfs on import thing = require('thing') even though babel is supposed to compile it away
You might need to enable the transformMixedEsModules option from @rollup/plugin-commonjs
https://github.com/rollup/plugins/tree/9874740ca685b1600e4319365c21e8c1131e9d95/packages/commonjs#transformmixedesmodules
(just rebased after #12636, I'm not currently planning to work on this)
This PR is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days.
Green! 🎉
On my machine this shaves off about 10% when running yarn jest packages/expect in this repo.
main:
Time (mean ± σ): 3.457 s ± 0.351 s [User: 4.286 s, System: 0.496 s]
Range (min … max): 3.249 s … 4.429 s 10 runs
This PR:
Time (mean ± σ): 3.151 s ± 0.060 s [User: 3.684 s, System: 0.480 s]
Range (min … max): 3.073 s … 3.249 s 10 runs
Turning on minification seems to improve the results slightly.
Time (mean ± σ): 3.130 s ± 0.077 s [User: 3.650 s, System: 0.495 s]
Range (min … max): 3.019 s … 3.248 s 10 runs
Hard to tell if that's any real improvement or not, tho. And having minification caused quite a bit of tests to fail (I suspect it's due to munging, but not 100% sure) so I'd like to explore that in a separate PR anyways.
Since we use exports I'm thinking this is not a breaking change. If people wanna use something internal that's not exposed we can revisit (and either revert until Jest 30 or expose whatever it is people are using).
Deploy Preview for jestjs ready!
Built without sensitive environment variables
| Name | Link |
|---|---|
| Latest commit | 06038a24eff9631ac07186fbd2b934166dc3158c |
| Latest deploy log | https://app.netlify.com/sites/jestjs/deploys/6509fe5fa3a7050008dcd7a5 |
| Deploy Preview | https://deploy-preview-12348--jestjs.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
A full test run on my machine goes from ~88 seconds to ~73 seconds (without transform cache). Promising 🙂
A full test run on my machine goes from ~88 seconds to ~73 seconds (without transform cache). Promising 🙂
I was investigating another issue and found that it seems debugging is not working, we are not generating source maps for bundled. I'm not sure if the increased speed has anything to do with it. 🤦♂️
To be honest, I'm not sure what I should do. In theory it would be more convenient to not bundle when developing locally, but this would require keeping both build processes and having the tests accept their line numbers. Do you have any suggestions?
Happy to produce sourcemaps, but it breaks some tests with filenames in the snapshots. Probably worth it?
If we also bundle during development, then just emitting the source map all the time should not break tests.
But there is still a problem in this case. Maybe we should not let users load the source map by default, which may cause us to test two situations at the same time.
Yep.
Debugging is broken due to using workers, I think. https://github.com/jestjs/jest/pull/14085#issuecomment-1689988616
I'm not sure if what I'm experiencing is related to this, I've been debugging under multi-threading before this. 🤦♂️
While I can put breakpoints in the tests, it doesn't work in the jest source code unless I put the debugger manually.
I just worked with the debugger now while putting together https://github.com/jestjs/jest/pull/14578 - breakpoints in jest sources worked fine. I use --inspect-brk and hit Continue once the debugger connects, maybe that's the difference? No manual debugger statements
Unfortunately I still can't get a break point in the ts file.
Ah, within the ts file! That probably needs source maps. I always debug in the chrome dev tools, never within the IDE. So haven't noticed 😅
Yeah. VSCode's debugger can automatically attach to child processes. :)
I'll try to get the tests to accept the source map, it shouldn't be too complicated,
Awesome!
We have some "absolute path to relative path" stuff for stacktraces, and stripping internal frames. That's where I'd assume any issues would arise - beyond that it should be fine 🙂
One regression: https://github.com/nodejs/cjs-module-lexer/issues/88
In practice, this breaks jest-watch-typeahead.
Unfortunate that TS types don't have a default export of all the things - not sure how to reconcile named exports in type exports with broken named exports in native ESM...
Fix for the above was to have our own ESM exports, bypassing the lexer entirely: #14661
Released in https://github.com/jestjs/jest/releases/tag/v30.0.0-alpha.1
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.