-
Notifications
You must be signed in to change notification settings - Fork 14
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
Type checking takes too long to be a precommit hook #1160
Comments
Or maybe there is a subset of repos where |
Perhaps there is an adhoc solution that would help eliminate the nlogn time complexity, since for multiple repos, they are all running tsc -b. Could we create a function where given a repo list L, we determine the correct dependency ordering, order the list into Is it possible that there is a library out on the internet that has done something like this before? I can't imagine we are the only typescript usage with so many endpoints for compiling. |
My impression was that if I was using the transpile (with watch), that |
This is precisely what We will shut off JS file output for tsc -b and only have it output declaration files. But AFAIK the “type checking without emitting JS” can still be incremental. |
Tagging for dev meeting to ask if it's OK if to disable type checking in precommit hooks. |
I'm not sure I understand. If you commit to 4 repos, and they all depend on joist, all 4 of the pre-commit processes are still going to look at joist.
Do we feel like we are at the fastest that we can be? If you feel confident that we can't speed things up any more, then I'm fine removing it and utilizing CT, but it seems less than ideal. |
Yes, but our project references are set up to be incremental, so declarations for joist are only built/updated once.
I haven't investigated any of the ideas in https://github.com/microsoft/TypeScript/wiki/Performance. But I think some type checks are fast and some are slow, and I don't know how to know which will be fast or slow, or why the slow ones are slow. |
An emphatic "no" here. Type-checking is not done by transpile.js. If it's not done in a precommit hook, then we're relying on the developer to run |
Keeping the type check sounds reasonable to me. And if the developer has already run a type check, that will be cached (since we use incremental tsc) and the type check during the precommit hook will be immediate.
I have not had this experience at all, my commits are running quickly for my current work. Maybe we can start taking notes on "which repos" and "which kind of changes" trigger that long type check? |
We may optimize this by implementing #1174 -- having one watch process, and the precommit hook just "checks in" with the watch process to see (a) is it running and (b) is it up-to-date with no errors? I also realized:
|
We discussed it today, we agreed it is OK to run the full type check as a precommit hook. We like the fast transpilation, and this seems like a reasonable compromise. |
@jessegreenberg said:
I replied:
@jonathanolson said:
I replied:
The text was updated successfully, but these errors were encountered: