TypeScript Contribution Diary: Trailing Type Parameters

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!

Next post: Pretty Error Counts

Parked shiny chrome trailer outside with trees and a hill in the background.

Medium told me to add a high resolution image, so here's the closest relevant thing I could find. ✔️

Problem Statement

TypeScript Issue 16152 (following Prettier Issue 1820): “trailing comma not allowed” error in type parameters. When you have a list of type parameters split across lines, some code styles and formatters would prefer you have a “trailing” comma after the last one to maintain consistency:

export const myLambda = <
	TFirstTemplate,
	TSecondTemplate // [ts] Error: Trailing comma not allowed.
>() => {
	/* ... */
};

That recently became a little more relevant because TypeScript added generic parameter defaults in addition to the existing extends clause. These giant syntax lists can be pretty hard to read unless split across lines:

export const createSourceNodeFromTemplateBinding = <
	TTemplateBinding extends TemplateBinding = TemplateBinding,
	TSourceNode extends SourceNode = SourceNode // comma here?
>(
	templateBinding: TTemplateBinding
) => {
	/* ... */
};

Something in TypeScript was complaining about those trailing commas.

Time to debug!

Step 1: Where

How does TypeScript create the complaint?

The complaint string was ”Trailing comma not allowed.”. I ran a full text search on that string to see where it comes from. There were a bunch of files, so I limited it to .ts files in the src directory. Exactly one result: src/compiler/diagnosticInformationMap.generated.ts. This file contains a giant key-value pair of lines with message-looking things. Spacing the one I found out for readability:

Trailing_comma_not_allowed: diag(
    1009,
    DiagnosticCategory.Error,
    "Trailing_comma_not_allowed_1009",
    "Trailing comma not allowed."),

Using the context clues from the “diagnostic” word in the file’s name and contents, this seems to be localized messages TypeScript can print out. 💯

Running a second full-text search on ”Trailing_comma_not_allowed” found exactly one method: checkGrammarForDisallowedTrailingComma, in src/compiler/checker.ts. From that name I was pretty sure this was what I needed. I also happened to remember reading in the (excellent) Basarat Gitbook how the ”Checker” in TypeScript is what validates types… and since this was in checker.ts, that felt good.

Step 2: What?

What does TypeScript do to create the error?

I ran a third full-text search on checkGrammarForDisallowedTrailingComma and found 5 usages in checker.ts. They were under these functions:

checkTupleType
checkGrammarTypeParameterList
checkGrammarTypeArguments
checkGrammarHeritageClause
checkGrammarVariableDeclarationList

A diligent programmer intent on learning all there is to know about TypeScript might go read and understand each of these… but I could tell from the name that checkGrammarTypeParameterList was probably what I needed.

function checkGrammarForDisallowedTrailingComma(
	list: NodeArray<Node>
): boolean {
	if (list && list.hasTrailingComma) {
		const start = list.end - ",".length;
		const end = list.end;
		return grammarErrorAtPos(
			list[0],
			start,
			end - start,
			Diagnostics.Trailing_comma_not_allowed
		);
	}
}

Without knowing much about node arrays, list types, or any of the surrounding code, it looks like this was checking if the list hasTrailingComma and adding a grammar error if so. Exactly what I needed! ✔️

Step 3: Fix

I removed calling checkGrammarForDisallowedTrailingComma from checkGrammarTypeParameterList, rebuilt the compiler (here’s how to rebuild the compiler), and pointed my VS Code to my built TypeScript (here’s how to use a local TypeScript in VS Code). It worked! No IDE complaints for source files with trailing commas on type parameters!

At this point, I was spooked: surely there must have been much more to do. TypeScript is huge! There must be overcomplicated cruft everywhere.

Turns out I was wrong: that was basically it.

Step 4: Verify

TypeScript validates these checker behaviors with “baseline” tests. Each test consists of a source file of TypeScript (or JavaScript) code under tests/cases/** and corresponding expected details under baselines/reference/**. Manipulating them is documented in their CONTRIBUTING.md.

I compiled & ran tests to discover exactly one failure:

Test: tests/cases/compiler/typeParameterListWithTrailingComma1.ts

class C<T> {}

The test complaint was that there should have been an error in this, as per tests/baselines/reference/typeParameterListWithTrailingComma1.errors.txt. After my change there was no longer an error.

💖 Perfect! 💖

I marked the baseline as accepted and the rest of the tests passed.

Step 5: Pull Request

Allowed trailing commas in type parameter/argument lists by JoshuaKGoldberg · Pull Request #20599:

This change is only one source file and one error file… there must be something I’m missing!? Fixes #16152

At this point I was still pretty worried about the change being purely subtractive. DanielRosenwasser (a core TypeScript team member) and vjeux (the top committer to Prettier) responded to my post with the 😄 emoji and my fears completely melted away.

Bonus points to vjeux and novjek for the 🎉 emoji. If you maintain any size repository, let this be a lesson for you: emojis make first-time contributing much less scary. Even though this wasn’t my first TypeScript pull request, it was my first time changing the language itself (or anybody else’s language), and I was feeling the imposter syndrome hard. This might sound like a joke, but it isn’t: humanizing the process made it feel like it was geared for outsiders like me and not just dedicated full-time paid closed-source maintainers. Even the customary thanks from mhegazy (core TS team member who merged the PR) felt nice. 😄

I happened to send the pull request in the middle of December, so it wasn’t until early January that it was merged in… but it got in without any complaints.

Takeaways

Find-in-Files is the only IDE command you need.
TypeScript is just like any large code base: daunting if you let it, approachable if you try.
Code maintainers with senses of humor are best. 🌹

Thanks for reading! I hope you got something out of this and to write more detailed posts on bigger contributions soon! 😅