-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Blob: Add types #19092
Blob: Add types #19092
Conversation
@@ -3,6 +3,9 @@ | |||
*/ | |||
const { createObjectURL, revokeObjectURL } = window.URL; | |||
|
|||
/** | |||
* @type {{[key: string]: File|undefined}} |
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.
Initially, my editor ignored the | undefined
part of this type and the following one. Adding explicit strictNullChecks
to the tsconfig restored it.
@@ -27,7 +30,7 @@ export function createBlobURL( file ) { | |||
* | |||
* @param {string} url The blob URL. | |||
* | |||
* @return {?File} The file for the blob URL. | |||
* @return {File|undefined} The file for the blob URL. |
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.
{?File}
is a nullable type in JSDoc, equivalent to File | null
, which isn't accurate.
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.
{?File}
is a nullable type in JSDoc, equivalent toFile | null
, which isn't accurate.
Yeah, I think you'll find a lot of examples like this in the codebase, where nullable type is used incorrectly as interchangeable with "optional" or "possibly undefined".
@@ -12,6 +12,7 @@ | |||
|
|||
/* Strict Type-Checking Options */ | |||
"strict": true, /* Enable all strict type-checking options. */ | |||
"strictNullChecks": true, |
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.
This should be enabled via strict
, but I some Type | undefined
types didn't seem to be handled correctly. Adding this seems to fix the issue.
I wonder if strict
+ noImplicitAny
results in odd behavior.
fefa371
to
9d8ae89
Compare
Description
Part of #18838
Extracted from #18942
Add types to
blob
package.Testing Instructions:
Verify type checking passes: