-
Notifications
You must be signed in to change notification settings - Fork 160
chore: improve test runtime by replacing ts-jest with swc #4336
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
Conversation
const baseTransformOptions = merge({}, swcrc, jestTransformOverrides); | ||
|
||
module.exports = { | ||
process: (src, path) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like Jest docs recommend implementing getCacheKey as well, although swc-jest transformer also doesn't seem to have it. Do you know if this would provide us any benefits?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent question; I didn't look at the jest transformer docs at all to be honest, I was mostly adapting from swc-jest's transformer (it would be better to contribute .swcrc support back to them, but I'm blocked on legal approval to sign their CLA before I can do that). I'll investigate this after standup!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed offline; will investigate improvements to the transformer in a separate PR (ideally against the swc-project/jest repo directly)
This reverts commit eb41b6f.
Details
Profiling our unit tests showed that the overwhelming majority of time spent was in
ts-jest
parsing typescript files. This PR is an experiment in replacingts-jest
with an alternative typescript transpiler,swc
, which is less mature but much faster.Motivation
On my local machine (a Surface Laptop 3 running Windows 21H1 OS Build 19043.985), comparative timings are:
main
(ts-jest)yarn test -- -- --coverage=false -- footer-section.test.tsx
yarn test -- -- -- footer-section.test.tsx
yarn test
On our CI build agents:
main
(ts-jest)Context
This is not as clear cut a transition as the timings above suggest.
swc
is a relatively young project; it's not quite as firmly single-maintainer asesbuild
, but it's still got a fairly concerning bus factor compared totsc
orbabel
(whatts-jest
hands off to). Still, the timings speak for themselves and it's easy enough to transition back if that ever becomes necessary.If this experiment goes well, we might consider a similar change to update our webpack config to use
swc-loader
instead ofts-loader
.One of the reasons
swc
is so much faster is that it does not perform type-checking. Right now we do type-checking as part of build, but if we were to switch webpack to use swc, we'd then need a separate build step for type-checking (comparable to the currentyarn null:check
step).Implementation notes:
.swcrc
file, which unfortunately doesn't support comments:noInterop: true
flag is the equivalent of TypeScript's defaultesModuleInterop: false
behavior. It is necessary; we use several modules which cannot be used correctly with interop behavior in play. See commonjs wildcard imports only expose enumerable properties of requirees' exports swc-project/swc#1778 for details."env": {}
property is necessary to triggerswc
to respect thebrowserslist
entry inpackage.json
swc
to target a reasonably recent set of browsers (via the newbrowserslist
entry inpackage.json
) is required; there are several tests with snapshots that change under default targeting, and there are a few components that break outright.tsx: true
is required (it tellsswc
to enable support fortsx
files; it doesn't prevent non-tsx files from parsing)@swc/core
. The swc project maintains a@swc/jest
transformer, but No way to override jsc or module swc configuration? swc-project/jest#8 prevents it from being able to use thenoInterop: true
setting we require, and I can't submit a PR to patch the project until legal rubber stamps their CLAcollapsible-script-provider.tsx
required a hacky workaround for swc incorrectly discards export statements and (most) comments swc-project/swc#1165--coverage=false
emitted javascript with different whitespace.collapsible-script-provider.test.ts
needed to be updated to format the resulting js before snapshotting it.src/scanner/processor.ts
used a mutable global in a module to enable its tests to inject test data. This confused swc; I replaced it with a more typical ctor injection pattern.browser-adapter-factory.test.tsx
did a dance around animport 'webextension-polyfill-ts';
statement to trick the module into thinkingwindow.chrome.runtime.id
is defined (it checks for this during import and refuses to load if it is not set). The dance we used is not compliant with how ES6 imports are defined to work (they are defined as hoisting import statements to the top of the file, ie, before we stub the window property). TypeScript has an outstanding PR/issue to stop supporting what we were previously doing (Hoist import declarations when transforming to commonjs/amd/umd module TypeScript#39764) and swc intentionally doesn't support it (Import statements are hoisted when target is CommonJs swc-project/swc#1686) (babel doesn't either), so I had to modify how our dance worked by moving it to a separate file and having it use a rawrequire()
rather than animport
.Pull request checklist
yarn fastpass
yarn test
)<rootDir>/test-results/unit/coverage
fix:
,chore:
,feat(feature-name):
,refactor:
). SeeCONTRIBUTING.md
.