-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
deps: update to typescript 3.5.3 #9357
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -182,11 +182,9 @@ function cleanFlagsForSettings(flags = {}) { | |
const settings = {}; | ||
|
||
for (const key of Object.keys(flags)) { | ||
// @ts-ignore - intentionally testing some keys not on defaultSettings to discard them. | ||
if (typeof constants.defaultSettings[key] !== 'undefined') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I seem to remember being very careful about the PR (#4960) doesn't seem to suggest there was anything special about this, and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
yeah, I was thinking since it's just the constants this should be ok (we won't have set anything to |
||
// Cast since key now must be able to index both Flags and Settings. | ||
const safekey = /** @type {Extract<keyof LH.Flags, keyof LH.Config.Settings>} */ (key); | ||
settings[safekey] = flags[safekey]; | ||
if (key in constants.defaultSettings) { | ||
// @ts-ignore tsc can't yet express that key is only a single type in each iteration, not a union of types. | ||
settings[key] = flags[key]; | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -329,7 +329,7 @@ class ReportUIFeatures { | |
*/ | ||
onCopy(e) { | ||
// Only handle copy button presses (e.g. ignore the user copying page text). | ||
if (this._copyAttempt) { | ||
if (this._copyAttempt && e.clipboardData) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've seen this error for a long time in VS Code. Did only newer TS emit this error? maybe my editor is set to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
they change the browser definitions from time to time, so it's possible. If you do the "Select typescript version" command in vs code do you have "Use VS Code's version" selected? |
||
// We want to write our own data to the clipboard, not the user's text selection. | ||
e.preventDefault(); | ||
e.clipboardData.setData('text/plain', JSON.stringify(this.json, null, 2)); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
caused by microsoft/TypeScript#30552. See that thread for the drama over strictness :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow, that is some good drama. I personally found microsoft/TypeScript#30825 more enjoyable to follow :)
Kinda nuts it's a useless version of
Omit
for me, I got excited for a second I wouldn't have to copy this everywhereThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
microsoft/TypeScript#30825 (comment) lol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh yeah, whoops, that's a way better link