Skip to content

Conversation

rbuckton
Copy link

This adds baseline and diff support for JS emit, fixes a few crashes, and accepts the current set of baselines.

@jakebailey
Copy link
Member

Hm, seems like this is causing us to check every file twice:

image

I take it this is because noCheck doesn't work, so the "repeat" emit test for that doesn't proceed quickly? Perhaps we should just skip this test for now?

@rbuckton
Copy link
Author

I take it this is because noCheck doesn't work

I think noCheck works. This is likely due to a typo in the Repeat implementation, I'll have a fix up shortly.

@jakebailey
Copy link
Member

It's improved, though we still do seem to do full checks? This is the thing Anders noted about --noCheck, that it doesn't skip as much as Strada.

@jakebailey
Copy link
Member

I don't really know what to do to review the files GitHub won't load, but it passes! 😅

@jakebailey
Copy link
Member

I did some local futzing to undo all of the baselines and get a diff of just the code. I think things look good, my only commentary would be about OutputRecorderFS; is the writable test FS not sufficient for the testing? I had to do some goofy stuff to make it compatible with symlinks so I wasn't sure if the Realpath stuff would work like that on non-existent files either (but, hopefully should). I don't think it's a big deal for now, though; I'll approve this if you want to start working with it.

@rbuckton
Copy link
Author

is the writable test FS not sufficient for the testing?

Is it possible to differentiate between what was present in the FS before compilation vs. after? In Strada we use a fake CompilerHost which keeps track of outputs.

@jakebailey
Copy link
Member

Not directly or conveniently, but @iisaduan's tsc tests track this by walking the FS before/after to show additions, removals, changes, etc, I believe.

@rbuckton
Copy link
Author

The vfs in Strada (harness/vfsUtil.ts) had that capability via snapshotting, but IMO the most reliable way to do this is to ensure a given component only interacts with a mockable host, e.g., instead of exposing FS on CompilerHost, relevant FS operations (UseCaseSensitiveFileNames, WriteFile, etc.) should be accessible via CompilerHost itself so that that dependency can be mocked as needed, not unlike what we're doing with cachedCompilerHost in harnessutil.

@rbuckton rbuckton added this pull request to the merge queue Mar 18, 2025
Merged via the queue into microsoft:main with commit 7a99cd9 Mar 18, 2025
21 checks passed
zshannon pushed a commit to zshannon/typescript-go that referenced this pull request Oct 6, 2025
zshannon pushed a commit to zshannon/typescript-go that referenced this pull request Oct 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants