-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Eliminate tests/lib/lib.d.ts, fix up fourslash to use real libs and defaults
#63056
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR modernizes the fourslash test infrastructure by eliminating the custom tests/lib/lib.d.ts file and implementing proper lib file resolution using real TypeScript library files. The changes ensure fourslash tests now correctly load and use the standard lib files, while also adding performance optimizations through caching.
Changes:
- Removed the custom
tests/lib/lib.d.tsfile that was preventing proper lib resolution - Implemented proper lib file mounting and caching for fourslash tests
- Added support for
// @libcompiler options in fourslash server tests - Fixed lib serialization to handle lib files correctly in inferred project options
Reviewed changes
Copilot reviewed 47 out of 780 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/harness/fourslashImpl.ts | Removed custom lib loading logic and switched to using VFS-mounted real libs |
| src/harness/harnessLanguageService.ts | Added lib file caching via selective document registry and VFS mounting |
| src/harness/vfsUtil.ts | Created createFourslashVfs to mount libs at the fourslash location |
| src/harness/harnessIO.ts | Removed obsolete lib file caching implementation |
| src/server/editorServices.ts | Added lib short name to full filename conversion for protocol options |
| src/harness/tsserverLogger.ts | Changed lib file text sanitization to read from built/local instead of compiler cache |
| tests/baselines/reference/* | Updated baselines to reflect lib.es5.d.ts instead of lib.d.ts |
| src/testRunner/unittests/helpers/contents.ts | Updated to use fourslashLibFolder instead of harnessSessionLibLocation |
|
I'll note that after this, there's no reason to have |
In my effort to try and bump our target (which then bumps a minimum lib, sort of), I kept hitting very strange behavior with fourslash tests and things that mentioned
lib.d.ts.tests/lib/lib.d.ts, which is some sort of weird copy of ES5 types with random stuff stuck in. If we raise the target, things are not happy about this file. So, I deleted it.lib.d.ts. This means that fourslash tests had not actually been loadinglib.dom.d.tsand so on, even though they probably should have been.libresolution, and then added them into test as ScriptInfos, only adding in what the test appeared to be using. Except tests could totally use different libs, and then they would silently not load libs they intended to. So I eliminated this and just made fourslash mount the libs like everything else.dom. So, I added caching to fourslash tests for lib files, which actually sped up the tests from 2m20s to 2m2s on my machine. Yay.lib: es5.// @...style options at all, unlike tsgo, which sets the inferred options. So, I fixed that too.libfiles. So, I fixed that too...So, at the end of all this, we're basically testing the same thing, but with much less cruft, and much much closer to Corsa, which will probably help with testing there.
But, the main thing is that now lib resolution and options handling actually respects the options that get set, there's no extra lib file that breaks when you use a newer target, etc, which I hope unblocks me trying to get the new target changes in.