-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Cache results for readFile, fileExists, directory exists, sourceFiles for .d.ts files across the build (only first time) #28629
Conversation
… for .d.ts files across the build (only first time)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with this code, but I have some questions after reading it.
… has its own cache logic).
@sheetalkamat may I ask you what changed your decision from "I don't think we want this in compiler host." and "You can move this to tsLoader to cache this instead of of affecting usage of tsc" in #27068 (review) to make this PR? |
@timocov This is not part of |
@sheetalkamat But my changes wasn't in |
@timocov Sorry about not reviewing changes after you moved it to tsc. I must have missed that. I remembered your PR only today when this got merged. |
@sheetalkamat but I didn't move they - they were there all time without any changes 🙁 |
Not cool behaviour of copying ideas from PR 😒 The right way was to add commits to @timocov to save his name in project history. |
Hey all, I'm not sure how best to address the what happened here, but I just want to reinforce that there was no ill-will or bad intent. We weren't trying to copy anything, and it really wouldn't make any sense to do so. It's inefficient and we want to make sure that people can make contributions to the project and know they had a part in it. This was clearly an oversight on our part, so I'm sorry about that @timocov. The TypeScript issue tracker gets a lot of traffic. Additionally, our team has recently had several team members move to different positions or go on parental leave, so we've had a lower headcount than usual. Covering all the bases and iterating quickly is hard, and while we were never perfect, it's been harder over the last few months. We'll try to be more conscious of this, but if you have ideas on how we can try to avoid this in the future we'd like to do better. |
There's some fishy stuff going on in here. |
This PR adds support to cache fileExists, directoryExists, readFile(.json files) and getSourceFile (for .d.ts). Currently this is done only in --build mode (and only first build in --watch mode but not incremental build) since that needs to be handled separately with invalidating files/directories as per watch.
The result running this against branch: