Skip to content

Conversation

@jakebailey
Copy link
Member

@jakebailey jakebailey commented Jan 28, 2026

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.

  • We have 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.
  • Then I discovered that fourslash tests were unconditionally using this file instead of the real lib.d.ts. This means that fourslash tests had not actually been loading lib.dom.d.ts and so on, even though they probably should have been.
  • Then I discovered that fourslash tests reimplemented the lib resolution, 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.
  • Then I discovered that the tests got 2x slower if we did this, because of all of the new files added via 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.
  • Then I discovered that the fourslash tests baselined so much that actually resolving the correct libs added 9 million lines of baselines. So I had to force every test that changed back to lib: es5.
  • Except, then I discovered that fourslash server tests don't repsect // @... style options at all, unlike tsgo, which sets the inferred options. So, I fixed that too.
  • Except, the code that sets inferred options has to be serialized info, which was never written to handle lib files. 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.

Copilot AI review requested due to automatic review settings January 28, 2026 03:10
@github-project-automation github-project-automation bot moved this to Not started in PR Backlog Jan 28, 2026
@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jan 28, 2026
Copy link
Contributor

Copilot AI left a 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.ts file that was preventing proper lib resolution
  • Implemented proper lib file mounting and caching for fourslash tests
  • Added support for // @lib compiler 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

@jakebailey
Copy link
Member Author

I'll note that after this, there's no reason to have @libFiles at all; the only uses are for react.d.ts which can be loaded via a triple-slash reference just fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

Status: Not started

Development

Successfully merging this pull request may close these issues.

2 participants