-
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
Prune tsconfig/all/tsconfig.json #1241
Comments
On lenovo laptop with AMD Ryzen 7 5800H
|
MSI desktop with Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz
|
@jessegreenberg reported 2m33.717s and 0m4.211s for the legacy after clean and with no changes, on |
I identified that the pruned tsconfig above won't be checking the demo applications in common code repos, like sun-main, etc. since it is not included transitively. But adding that explicitly for all those repos didn't significantly change the time on Mac. Before: 0m25.886s I debated whether to request review before or after committing this to master. It seems a step in the right direction, and my local tests are working well, so perhaps I'll commit. I'll mark blocks-publication and reach out for a reviewer. |
I wrote to our typescript slack channel:
It seems to be working nicely on my side, and I pushed the change to master. Can someone please volunteer to review? Also, please review https://github.com/phetsims/chipper/blob/master/tsconfig/all/tsconfig.json and confirm the sim(s) you are currently working on are listed in that file. The riskiest part of this change is if a sim got dropped from that list. I generated that list by (a) sims that have a *-main.ts file or common code repos. Hopefully nothing was missed, but please add anything that was missed. |
Also, if this disrupts your work for any reason and cannot be resolved quickly, please feel free to report and revert. |
Also, I realized this doesn't speed up the precommit hooks at all, since they run |
@zepumph asks if |
@zepumph and I discussed getting rid of all the tsconfig files. That seems to have a good performance characteristic and we could get rid of a lot of complexity in our tsconfigs. The lint rule project could point to the monolithic one? |
On hold until we discuss #1245 |
The new monolithic transitive tsconfigs have been working great, closing. |
When we started on TypeScript, we used
tsc
to transpile files. Now we use babel for transpiling files, and only want to usetsc
for type checking. We also have"checkJs": false
in our tsconfig-core. Therefore, we no longer need to list all repos in all/tsconfig.json. This has the potential to speed up our type checking time, which has been a hassle.Let's run before/after testing on the timing. I'll also compare to a monolithic tsc, which doesn't use project references at all.
The legacy tsconfig is in master.
Here is the pruned version:
And here is the monolithic one:
Running on Macbook Air M1
something1?: string;
in NodeOptionsthis.age=7;
in GOSim.tsIt seems like the next step is to performance test on Windows.
The text was updated successfully, but these errors were encountered: