Fix more newtab/lib TypeScript errors so 11 more files can be type-checked
Categories
(Firefox :: New Tab Page, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox160 | --- | fixed |
People
(Reporter: jaws, Assigned: jaws)
References
Details
(Whiteboard: [hnt])
Attachments
(10 external files)
tsc can only check a file in browser/extensions/newtab/lib once it has no errors, so 16 files are still on the exclude list in browser/extensions/newtab/tsconfig.json. These patches fix the errors in 11 of them, mostly wrong or missing JSDoc, and take those files off the list. Four of the patches fix small real problems that tsc found, such as an error message that dropped the scheme it was meant to report.
Updated•1 month ago
|
| Assignee | ||
Comment 1•1 month ago
|
||
The scheme was passed as the second argument to new Error, which is ErrorOptions, so the logged message ended in "but got " with nothing after it; tsc reported this as TS2559, and the file now leaves the tsconfig exclude list.
| Assignee | ||
Comment 2•1 month ago
|
||
willDoomOnShutdown used Array.prototype.find, so it returned the matching nsIURI or undefined rather than the documented boolean; every current caller only tests it for truthiness, so this changes no behavior, but tsc reported it as TS2322 and using some makes the return value match the contract.
| Assignee | ||
Comment 3•1 month ago
|
||
WeatherFeed.fetch takes no arguments, so the isStartup flag that init passed to loadWeather and that loadWeather forwarded to fetch was never read; tsc reported the extra argument as TS2554, and the file now leaves the tsconfig exclude list.
| Assignee | ||
Comment 4•1 month ago
|
||
Bug 1953330 changed BUILT_IN_SECTIONS to take no arguments, but SectionsManager.init and addBuiltInSection kept reading the newtab and pocketNewtab Nimbus variables to pass to it; tsc reported the extra argument as TS2554, and the file now leaves the tsconfig exclude list.
| Assignee | ||
Comment 5•1 month ago
|
||
skipMain is only passed by OnlyToOneContent, so the other callers in NewTabInit and WebNotificationsFeed were flagged for passing two of three arguments; NewTabInit now leaves the tsconfig exclude list.
| Assignee | ||
Comment 6•1 month ago
|
||
The JSDoc for these functions omitted properties the code reads or passes: manifest in RemoteRenderer's content objects, endpointUrl in two TemporaryMerinoClientShim fetches, the options of WebNotificationsFeed._dismissAll and skipNightlyDefault on WidgetRegistryEntry, while AdsClient.uninit's client was documented as a string; AdsClient, TemporaryMerinoClientShim and RemoteRenderer now leave the tsconfig exclude list.
| Assignee | ||
Comment 7•1 month ago
|
||
STORIES_REGION_LOCALE_DEFAULT is now typed as RegionLocaleEntries, with a cast on its Nightly-only spread since tsc infers that conditional array as nested string arrays rather than tuples, and normalizeDiskEntry casts its accumulator because id is copied in from DISK_PASSTHROUGH_FIELDS after construction; WebNotificationsFeed now leaves the tsconfig exclude list.
| Assignee | ||
Comment 8•1 month ago
|
||
widgetLabels is passed WIDGET_REGISTRY entries, whose prefsL10nId is optional because retired entries omit it, so the hand-written { id, prefsL10nId } type rejected them; AboutPreferences now leaves the tsconfig exclude list.
| Assignee | ||
Comment 9•1 month ago
|
||
The constructor copies DEFAULT_OPTIONS and its options argument onto the instance with Object.assign, so tsc never saw dispatch, pageURL or the message names as members and required callers such as Store to pass every option the defaults already supply; the options are now documented as optional, the four members are declared as typed class fields, and onMessage's JSDoc names msg.data instead of msg.action, the property it actually reads. ActivityStreamMessageChannel and Store now leave the tsconfig exclude list.
| Assignee | ||
Comment 10•1 month ago
|
||
The constructor destructured an undocumented options object that defaults to {}, and the impression and allocatedTask typedefs marked as required the timestamps and DAP task fields that onAttributionEvent's default impression never has; the file stays excluded for the ObjectStore.get error that comes from wrapMethods in toolkit/modules/IndexedDB.sys.mjs.
Comment 11•1 month ago
|
||
Comment 12•1 month ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/2e07df63abc5
https://hg.mozilla.org/mozilla-central/rev/e801f5b337da
https://hg.mozilla.org/mozilla-central/rev/bffb3b7e7537
https://hg.mozilla.org/mozilla-central/rev/48fed745891d
https://hg.mozilla.org/mozilla-central/rev/481cd2bcd711
https://hg.mozilla.org/mozilla-central/rev/49ac54118a35
https://hg.mozilla.org/mozilla-central/rev/bace8a49d8f9
https://hg.mozilla.org/mozilla-central/rev/ed3409e32a59
https://hg.mozilla.org/mozilla-central/rev/518569a24916
https://hg.mozilla.org/mozilla-central/rev/211b9baed96a
Updated•15 days ago
|
Description
•