Iâve started making contributions to TypeScript, working my way up from small error reporting nitpicks to (hopefully!) real syntactic and control flow improvements. These blog posts are documentation of the steps I took to figure out what to do for these contributes & then do them. If youâre thinking of contributing to TypeScript or other large open source libraries, I hope seeing how to delve into a giant foreign monolith is of some help!
Previous post: Trailing Type Parameters
Next post: Identifiers after Numeric Literals
Problem Statement
TypeScript Issue 22124: Provide summary of errors when in â pretty mode?
When refactoring large projects, the CLI output from compile runs (both from solo compiles and --watch
) can build up pretty quickly.
Every time I save a file it feels like the terminal is filling up with a giant sea of angry errors.
It would be convenient to have an error around the lines of âFound 43 errors in 21 files.
â at the bottom of each compile to help track progress.
Hereâs what a --pretty --watch
run with errors looks like now:
Time to debug!
Step 1: Where
Where should TypeScript print the list of error messages?
Presumably, there must be something that prints out each of the error messages in order.
The messages only have some of this fancy formatting in --pretty
mode, so I figured itâd be easier to look for whatever prints that stuff and see who calls it.
Two unique, search-friendly features of --pretty
printing are:
~
underlines underneath error messages- Special console colors
I ran a search on â~
â and found a few places in src/compiler/program.ts
, all within a function named formatDiagnosticsWithColorAndContext
.
Perfect!
I put a console.log("Found", diagnostics.length, "errors");
at the bottom of formatDiagnosticsWithColorAndContext
and ran the compiler on a project with two errors across two files to see the message being printed twice:
So it looks like formatDiagnosticsWithColorAndContext
gets run on every file.
Not on every run.
Not so perfect.
đŠ
A full-text search showed it only ever being called in createDiagnosticReporter
in src/compiler/watch.ts
.
That is called by createWatchCompilerHostOfConfigFile
, which passes it as a reportDiagnostic parameter to createWatchCompilerHost
.
Within that function itâs only ever used within a new function named emitFilesAndReportErrorUsingBuilder
, which passes it to emitFilesAndReportErrors
.
I put the same console message in the new location and again ran the compiler on the same project. This time there was a single message at the end about two complaints:
Hooray! Found the place.
Step 2: How?
How to print all the errors?
I wanted to print both the count of errors and count of files. The first part seemed to already be solved by the count of diagnostics. Remaining to do was to count the unique file names from the diagnostics, create some kind of diagnostic message about the results, and print that message.
2.1: Counting Unique File Names
The Diagnostic
type has a field file: SourceFile | undefined
and the SourceFile
type has a field fileName: string
.
I figured that was a reasonable source of information on file names.
But counting them?
Counting unique objects is generally optimally efficient in JavaScript with a Set.
I ran full-text searches on new Set
and Set.from
but didnât find anything⊠which makes sense, as TypeScript works in environments that donât natively have Set.
But surely there must be something in TypeScript for counting unique objects? What about the slightly-older Map?
I tried again searching for new Map
and found two generic, useful-looking functions at the top of src/compiler/core.ts
:
createDictionaryObject
createMap
Maps are useful when you want a dictionary-like object but canât guarantee you wonât have have to check edge case key names such as â
hasOwnProperty
â. That tends to happen when youâre coding a superset of JavaScript.
createMap
uses createDictionaryObject
internally to satisfy the Map
interface.
Looks like TypeScript polyfills Map
.
I havenât personally executed reliable performance tests to evaluate which is faster, but if TypeScript has a function to choose between the two options, I figured it would make sense to go along with whatever TypeScript used.
Conclusion: use createMap
to keep track of unique file names with errors.
2.2: Localizing Error and File Counts
The diagnostic messages stored by TypeScript in diagnosticInformationMap.generated.ts
are string templates without any logic for pluralization logic.
Thatâs a little inconvenient as I wanted to print a different message in each of three singular/plural possibilities:
- Single error, single file
- Multiple errors, single file
- Multiple errors, multiple files
Three new strings it was.
The contribution guideâs section on localization mentions that those strings come from src/compiler/diagnosticMessages.json
and are generated by the jake generate-diagnostics
command.
For example:
"Compilation complete. Watching for file changes.": {
"category": "Message",
"code": 6042
},
The âcode
â field is interesting.
There are codes around that compilation message from 6001
to 6190
, then gaps around. I assumed this was some kind of organization strategy and added the three new messages in 6191
-6193
:
Found_1_error_in_1_file: diag(6191, ...)
Found_0_errors_in_1_file: diag(6192, ...)
Found_0_errors_in_1_files: diag(6193, ...)
Later, I thought to click the Diagnostics guidelines link in CONTRIBUTING.mdâŠ
2.3 Diagnostics Printing
Other diagnostics in emitFilesAndReportErrors
are printed via a function named reportDiagnostic
.
I tried using that to print the new summary message but got a different-looking message prefix:
Elsewhere, such as with the nice âCompilation complete. ...
â message, the reportWatchDiagnostic
function not available in emitFilesAndReportErrors
was used.
It was being created locally within createWatchCompilerHost
but not passed through to (or visible within) emitFilesAndReportErrors
.
I changed emitFilesAndReportErrors
to take it as a parameter (in addition to system.newLine
to pass to the reporter
) and use it:
Great that it printed!
Butâ that screenshot of just the two messages was everything.
Why no "Starting compilation in watch mode..."
?
I went up the call chain for printing messages through createWatchStatusReporter
and found a clearScreenIfNotWatchingForFileChanges
function clearing the screen on printing each diagnostic message if the message isnât the specific âStarting compilation in watch mode...
â one.
Spooky!
đ»
I didnât understand why this function was made that way (why not only clear the screen when starting a new compilation run?) and didnât feel comfortable changing it.
Spooky behavior like that felt best left alone and asked about in the pull request.
I added an array of allowed message codes to not clear on (not TypeScriptâs Map
shim because it only allows string
s as keys).
Step 3: Cleanup
emitFilesAndReportErrors was also being used by performCompilation in src/compiler/tsc.ts. I discovered this when I tried to compile and it yelled at me. That was a little surprising: why is the core tsc compiler using watch-specific logic? More spookiness⊠đ€
I couldnât pass the added reporter in from performCompilation
because thatâs of type WatchStatusReporter
, which is only available in the code that deals with watching logic.
That meant I would have to find a way to only report the new diagnostics in watch mode.
After hackily fumbling around with optional parameters for a while I ended up cutting & pasting emitFilesAndReportErrors
up into three functions:
getProgramDiagnosticsAndEmit
: Retrieves diagnostics output and emit results from the program.reportDiagnosticErrors
: Outputs the traditional error reporting that already existed.summarizeDiagnosticsAcrossFiles
: Outputs the new summary message.
tsc
calls the first two; watch
calls all three.
Step 4: Pull Request
Normally before sending out a PR I would write unit tests validating the change. Always test your changes! However, because I didnât understand the screen clearing behavior, I decided to put up what I had at this point for code review and ask there.
Added an errors summary for âpretty âwatch results by JoshuaKGoldberg · Pull Request #22254
The first commit in that PR has everything I mentioned above: e0d067b48f71e545c9bef49ad5fceb0a2a17a0ea.
Step 5: Feedback
Someone asked me to change things! This didnât happen in the last contribution diary articleâs pull requestâŠ! đ„!
There were a few big pieces of initial feedback:
- Diagnostics arenât necessarily attached to files, so it would be inaccurate to report them as across files.
- Not all diagnostics are errors, so it would be inaccurate to count them all in the errors count.
- It would be cleaner to just add another optional parameter to
emitFilesAndReportErrors
that reports the error summary.
5.1: Removing File Counts
Not needing to report file counts actually made my change easier. I removed the function that counted unique files with a Map. It was a little upsetting at first to not have needed knowledge from the createMap investigation⊠but on the bright side, I now knew more about TypeScriptâs internals than I would have otherwise. Thatâs a good thing! đ
5.2: Filtering Errors
I had to ask for confirmation on how to filter out non-error diagnostic messages. The method was pretty simple:
diagnostics.filter(diagnostic.category === DiagnosticCategory.Error);
đ Protip: never be scared to ask for clarification on pull requests! đ
5.3 Optional Error Summarizer
Specifically, the feedback was:
I would rather modify the existing
emitFilesAndReportErrors
to take additional parameter to summaryReporter which could either beundefined | reporter
and if provided you write the summary using that reporter.
An excuse to undo all the changes in tsc.ts
!
Perfect.
5.4 Miscellaneous Feedback
There were a couple small points left, but nothing major. One was a preemptive bug fix (fixed đ) and the other was a unit test comment (unable to do for a technical reason).
A merge from master, a couple of updates, and the PR was accepted! đ
Takeaways
- Find-In-Files, Find-All-References, Go-To-Definition are still best. đ„
- Itâs good to learn things, even if you donât use that knowledge immediately.
- Reviewers want you to write your best code for the project. Listen to them!
- Donât be afraid to ask for clarifications of reviewer comments if you need.
- Donât be afraid to start a dialog with reviewers if you need.
Thanks for reading! I hope you got something out of it! đ