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

perf: use swc or esbuild toolchain to transpile code locally #314

Closed
varunsrin opened this issue Dec 21, 2022 · 4 comments · Fixed by #330
Closed

perf: use swc or esbuild toolchain to transpile code locally #314

varunsrin opened this issue Dec 21, 2022 · 4 comments · Fixed by #330
Labels
help wanted Well specified and ready to be worked on t-chore Miscellaneous improvements

Comments

@varunsrin
Copy link
Member

What is the feature you would like to implement?

All local actions (yarn start, yarn test) should use a fast transpiler like esbuild or swc, and remote CI actions should compile the code at most once before running tests.

Why is this feature important?

Faster build times makes it easier to work with the Hub codebase

Will the protocol spec need to be updated??

How should this feature be built? (optional)

Currently, we use tsx for yarn start which uses esbuild under the hood, and nothing special for yarn test. In CI, we run yarn tsc and then yarn test with some CI flags. Ideally, we'd do a full typecheck in CI but when running locally we'd just do a fast transpile and execute. We may also be double compiling in CI adding to our run times, needs investigation.

Additional context

There is some complexity due to ESM where additional configuration is needed to get certain packages to work. For example, swc is a drop in replacement in jest, but esbuild requires some fiddling with configurations. Similarly, tsx was the only drop in replacement for yarn start that actually worked.

@varunsrin varunsrin added help wanted Well specified and ready to be worked on t-chore Miscellaneous improvements labels Dec 21, 2022
@deodad
Copy link
Member

deodad commented Dec 26, 2022

Looked into this, here's my recommendation:

  • setup swc to replace ts-jest
  • leave tsx for running dev server

This solution achieves the best performance w/ minimal configuration.

Ideally we wouldn't need two different transpilers but going with either one for both test and start involves adding undesirable configuration complexity and slight performance warts:

  • swc is easy to configure w/ jest and ts-node (ts-node is used by jest to transpile `jest.config.ts) but setting up a dev server requires a workaround that involves maintaining a custom node loader until this issue regarding esm + path aliases is resolved
  • esbuild is already working via tsx for start, but I wasn't able to get jest transpiling the project w/ esbuild using any of the existing transformers. getting it working for this project would likely require writing a custom transformer. it also doesn't integrate w/ ts-node, so there's the tiny wart of taking 1+ second just to load jest.config.ts

lmk if this sounds good and I'll open up the PR

@varunsrin
Copy link
Member Author

How much faster does this hybrid approach make us when running tests from a cold boot locally? If the savings are meaningful, I'm supportive otherwise I don't think the additional complexity of having multiple build setups is worthwhile.

I would also document this rationale in the code in the config files or somewhere obvious, so that it's clear to future maintainers why we chose this hybrid approach. You can just link to this ticket or copy paste the info there if its short enough.

@deodad
Copy link
Member

deodad commented Dec 27, 2022

yarn test goes from ~43s to ~20s on my machine
yarn test ./src/utils/crypto.test.ts from ~5.8s to ~2.4s

I opened up a PR to see how it performed in GH Actions:
shard 1/2 went from averaging ~45s to ~40s
shard 2/2 went from averaging ~70s to ~53s

Github hosted runners are only 2-core by default so the slowest tests end up being a bigger factor.

@varunsrin
Copy link
Member Author

varunsrin commented Dec 27, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Well specified and ready to be worked on t-chore Miscellaneous improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants