core(tsc): update to latest tsc #5581
Conversation
brendankenny
requested review from
ebidel,
patrickhulce and
paulirish
as code owners
|
sorry spammers, I made a real PR out of the branch :P |
| @@ -226,7 +226,13 @@ class LighthouseReportViewer { | |||
| return new Promise((resolve, reject) => { | |||
| const reader = new FileReader(); | |||
| reader.onload = function(e) { | |||
| resolve(e.target && e.target.result); | |||
There was a problem hiding this comment.
why? is new ts stricter on this?
There was a problem hiding this comment.
technically it's somewhat more correct. They had a custom FileReaderLoadEvent passed to onload before, which always has the target set to the FileReader, but that isn't a real standardized event. They're moving to automatically generate the types from Edge's Web IDL files, so I think that's why that custom event was eliminated.
However, the event should be generic, parameterized on the element itself so that the type is known and it isn't just an EventTarget but the actual element, but they haven't done that. Sort of for performance reasons, but probably also partly because people aren't asking hard enough: microsoft/TypeScript-DOM-lib-generator#207
| @@ -65,7 +65,9 @@ async function runLighthouseInExtension(options, categoryIDs) { | |||
| throw new Error('no runnerResult generated by Lighthouse'); | |||
| } | |||
|
|
|||
| const blobURL = createReportPageAsBlob(runnerResult); | |||
There was a problem hiding this comment.
why does ts require us to do this now?
There was a problem hiding this comment.
runnerResult.report is string | Array<string>, but we were treating it as a string (which it is in this case, but the compiler doesn't know that). I'm not sure why it wasn't catching it before.
| @@ -11,6 +11,7 @@ | |||
| /** @type {Record<LH.Locale, LocaleMessages>} */ | |||
| const locales = { | |||
| 'en-US': require('./en-US.json'), // The 'source' strings, with descriptions | |||
| // @ts-ignore - tsc bug, something about en/en-US pointing to same file | |||
There was a problem hiding this comment.
this is a really weird bug. Any characters changed or added to either the en or en-US keys makes the error go away
includesordering hack in tsconfig@typetypes in a few places to the more standard (and readable)@param/@param/@returnform{}if we want them to default tovoid)Array<string>where astringis expected (never actually happens at runtime)@templatewith type constraints. In theory this allows us to bring back in all theDriver.on, etc methods that I moved out of class bodies to the end of the file, but for now I'm leaving in place (except for one) because typescript stopped inferring parameter types on things likeI'll have to investigate why that's happening (probably microsoft/TypeScript#1449), but really nice to have that ability now.