-
Notifications
You must be signed in to change notification settings - Fork 1.4k
chore: revamping build tooling #827
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
Closed
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as resolved.
This comment was marked as resolved.
This was referenced Apr 17, 2023
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
d6d9b64 to
db50558
Compare
This was referenced Apr 28, 2023
Contributor
Author
|
This was referenced May 3, 2023
Contributor
Author
|
Every improvement from this PR has been independently merged already. The final missing piece is here: So I'm closing this one. |
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
fuelsbuild #834This PR thoroughly revises the current build system, double-checks configs, rules, and caching mechanisms, removes
tsupin favor of good oldtsc, fulfills all requirements from #834, and does a few extra optimizations here and there.I will try to outline the main ones.
Local install (TL;DR)
More notably, you can now install and run your local versions of
fuels.This was the primary motivation that drove everything else.
Remember to
buildfirstSince our packages export Javascript files now, we need to build the packages after each change we make. Otherwise, we'd still be looking at outdated compiled Javascript files. To automate this, you can run the following:
# on repo root pnpm devThis will
pnpm buildthe project once, then watch and re-build it automatically on every change. It usesnodemonto watch all packages simultaneously for optimum performance. The building part continues to be handled by Turbo Repo.Go to Definition
To make it possible to export Javascript files and have a local working
fuelspackage while still being able togo to definitioninside our Typescript source files, we had to generatedeclarationMapsin our build routine:DeclarationMaps are SourceMaps for DTS files; they map it back to their source
*.tsfile.This is the secret hidden part.
Yes compile, no bundling
Because
tsupcan't generatedeclarationMaps, it was replaced by good ol'tsc, which means:Bundling was a feature of
tsup, whichtscdoesn't do.All packages are still published in CJS and ESM formats, but now as multiple individual files mirroring the source directory structure instead of a single
.jsfile. Frontend frameworks can easily tree-shake it as part of their bundling.Forc optimizations
Every time you do a
pnpm install, it triggers apreinstallroutine forpackages/forc-binthatwillwould download allForcbinaries. Again. All the time. Starting now, it first checks if it needs to download Forc and skips otherwise.Build optimizations
In the middle of every
pnpm buildfor theexample-contractpackage, it would trigger apnpm installagain. Since we use pnpm w/ workspaces, that install would occur on the repo root. Triggeringpreinstallhooks again, re-downloading Forc, and all that good stuff. It would delay the build by around ~10s or so, but not anymore.Before:
fuels-ts/packages/example-contract/package.json
Line 8 in dd7b1ca
fuels-ts/packages/example-contract/scripts/build.sh
Lines 5 to 9 in dd7b1ca
After:
fuels-ts/packages/example-contract/package.json
Lines 8 to 10 in 1c5c22f
Referencing the
fuels typegencommand using its full path circumvents the fact that thefuelssymlink might not have been created yet, during the firstpnpm install(which must be run right before the firstbuild).Because at first, our bin entries
"bin": "dist/bin.js"are not pointing anywhere because the packages were not compiled yet and the JS files don't exist. We firstpnpm installand thenpnpm build. But theinstalltries to create the symlinks for the files that will only exist after the first timepnpm buildis run.The last workaround was doing an
npm installonly to recreate these symlinks.Stop running tests twice
Instead of running tests twice for each PR (for computing coverage diffs between the base branch and the PR), we now save the base branch coverage as an artifact and download it on each PR, cutting test time in half.
Final Notes
Worths mentioning:
builds are a bit fasterdevmode (build-watch) is lot fasterbuild&testroutines combined are ~3mins fasterTotal CI time for
build+test:master~15m~12m~11m~6mMore PRs to come.