-
Notifications
You must be signed in to change notification settings - Fork 6
TS compilation improvements #3415
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
This allows scripts to be ran even if TS is mad, which is a lot of cases could fix the TS errors. This is available now that we are on TS 5.6+ We still run type checks separately in CI as well, so the checking here was a wasted step.
It is used for entry point's relative path; not just the cli generator commands.
This causes src files to be put into dist/src/ now. I've avoided this in the past, but I think it is fine now, as I'm intentionally doing it. This leaves room for other top-level folders as well.
📝 WalkthroughWalkthroughThis change updates build and execution configurations by modifying script paths and invocation methods in Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
💤 Files with no reviewable changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (11)
🔇 Additional comments (6)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
``` yarn gel:gen ``` now runs the standard ts compilation which generates the js files in dist/ Previously the gel user did not have permission to this dir, as we only applied the owner change to src/ (old line 87). Now we'll set the current user as "gel" and copy in all files as that owner. This is more idiomatic for Docker.
Transpilation vs type check
Now we do not check types on build/transpilation/script execution. This treats TS more as a linter, which is a good separation from transpilation. This allows scripts to be ran even if TS is mad.
This was a particularly painful with Gel - TS interaction.
We use the app to generate Gel access policies, which is needed to make Gel schema changes, and generate TS types from this new schema.
This cycle caused us to deadlock. We couldn't make Gel schema changes, because we needed APs. And we couldn't make APs because the app had model changes that were not yet in sync with generated Gel types.
This feature was added in TS 5.6 so this depends on #3410
Script execution
Now we use Nest & its standard TS setup, to execute scripts instead of
ts-node
.This is more straight forward, as obvious by the diff.
Now all TS files are incrementally compiled to the dist folder first, then commands are executed.
Somehow before I believe there were no pre-compiled cached js files, and so every execution needed to take that set for all files.
Now this pre-compiled cache (TS
incremental
builds) are actually effective.Scripts execute much faster now, between the
incremental
cache and the no type check on build setting.Note:
ts-node
is needed byjest
/ts-jest
, so it's been kept...for now 😈