2065670 - (tabbrowser-ts) [meta] Enable TypeScript for browser/components/tabbrowser

Status: NEW

(Firefox :: Tabbed Browser, task)

Description

[Dão Gottwald [:dao]]

This effort puts browser/components/tabbrowser under the tree's TypeScript setup and gates it in CI. The directory drives Firefox's tabs — opening and closing them, pairing each tab with the browser element that displays its content, moving them between windows, and the tab strip they appear in — across nine modules and fourteen scripts that run inside the chrome window. Its core is a single 10.9k-line class, Tabbrowser, that every part of the browser UI calls into, and the one piece the setup cannot check on day one. The JSDoc it carries lists every parameter, which the lint rules require of any comment that exists — but only a quarter of its members carried a comment at all, and nothing checked the types in the ones that did.

None of the machinery is new. Mark Banner built it for the address bar — the tree-wide TypeScript linter that gates every component, the compiler settings, and the convention for declaring types a checker cannot otherwise see — and this effort follows that road. This still poses a challenge as the Tabbrowser class is larger than anything the address bar effort put under the checker.

A checked contract catches defects that tests do not. The failures in this code are quiet ones: moving tabs between windows, inheriting a container, attributing an action to where it came from — paths that produce wrong state or wrong data rather than an error anyone reports, and that no test asserts on. Checking turns a mismatch on one of them into a review-time error instead of a field nobody notices is wrong.

Those failures stayed invisible because nothing verified the documented contract. Option bags listed properties no caller passes, a parameter documented as a number was handed a string, annotations named types that never existed. Each is something a reader trusts and a reviewer has no way to check.

The second gain is shared. Nothing could describe a tab, a tab group or a split view to a type checker, so every component that touches them either guessed or went unchecked. Writing that contract down once unblocked work in Session Restore immediately, and three partial copies elsewhere in the tree now have one declaration to converge on.

What the checker has caught already

A running list rather than an audit — ten of the fourteen steps have landed, two are underway, and two are still ahead.

• Pins from a multi-selection recorded their source as "unknown." A metrics context was passed where the receiving method expected an options object carrying one, so the telemetry attributed every such pin to nowhere. Fixed in Bug 2065803.
• The tab strip was holding a TaskbarTab, one of Web Apps' own storage objects. Naming that class in a comment meant asking Web Apps to export it, which is how the leak came to light; they are pulling the entry back behind their own module. Filed as Bug 2066310.
• Glean event extras were declared as text, so a caller passing a number or a flag was a type error. The recording interface does take text and converts whatever it is handed, so the callers were right and the declaration was too narrow. Saying so clears the same false complaint for every typed consumer of every event in the tree. Fixed in Bug 2069198.
• Several suspects that turned out to be documentation, not code. A closing method documented as taking two flags while callers pass six more straight through it, for one. The checker raises the question; the answer is often that the comment was wrong.

Progress at a glance

Phase	Goal	Bug	Status
Step 0	Move the class into a system module	2049770	DONE
Step 1	Repair a type definition a stray tag had truncated	2065656	DONE
Step 2	Land the project file green and wire the CI gate	2065668	DONE
Step 3	Expose two modules the class hung off its instance	2065759	DONE
Step 4	Make the annotations' type names resolve, and document the option bags	2065769	DONE
Step 5	Declare the tab strip's custom elements	2065814	DONE
Step 6	Fix the annotations that disagree with the code	2065813	DONE
Step 7	Hold the tab strip's shared preferences on the class	2069200	DONE
Step 8	Take the class to zero and put it under the gate	2069199	DONE
Step 9	Give the tab group, its label and the split view the members the class reads off them	2068697	DONE
Step 10	Convert the tab group and split view elements to modules	2066073	IN PROGRESS
Step 11	Convert the tab strip element to a module and type it from its class	2070849	IN REVIEW
Step 12	Convert the tab element the same way, retiring the last stand-in declaration	Not filed yet	PLANNED
Step 13	Move the class's remaining window-independent members to statics	Not filed yet	PLANNED

Checking it yourself

Two commands, and which one you want depends on what you are asking. To check the code while working on it, run the project directly and read the type checker's own output. The argument is the directory holding the configuration:

./mach ts check browser/components/tabbrowser

For CI's verdict, or for machine-readable output, run the linter, which is the command the gate runs, and which now covers the class itself on every push. It takes any path, maps it to the project containing it, and runs that whole project:

./mach lint -l typescript browser/components/tabbrowser

Both end up invoking the same type checker on the same configuration, so they agree on what is wrong. The mapping is the part worth remembering: a path belonging to no configured project runs nothing and reports success.

One more thing is worth knowing. The JSDoc here has three consumers that disagree about type syntax — the checker, the lint rules, and the generated API reference — so a change that satisfies one can silently break another: an array-of-union written one way drops a parameter from the rendered page without failing anything.

References

• Firefox source docs: TypeScript