Enable Typescript strict flag for Storage #1935
Conversation
| // TODO: This disable can be removed and the 'ignoreRestArgs' option added to | ||
| // the no-explicit-any rule when ESlint releases it. | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| function triggerCallback(...args: any[]): void { |
There was a problem hiding this comment.
Should we follow the same pattern in handler() and change the signature as well? Something like:
function handler(success:boolean, ...args:any[])
| @@ -30,7 +30,7 @@ import * as type from './type'; | |||
| * modified after this blob's construction. | |||
| */ | |||
| export class FbsBlob { | |||
| private data_: Blob | Uint8Array; | |||
| private data_!: Blob | Uint8Array; | |||
There was a problem hiding this comment.
Can you explain to me what the bang here means?
There was a problem hiding this comment.
It is a way of us promising the compiler that this variable will be initialized before anything calls it. The compiler thinks data_ isn't definitively set in the constructor because there's an if and 2 else ifs where it is set, but no else. Since the constructor requires the data param to be of a type that would meet all possible if conditions, it should always get set in the constructor.
I can fix it by adding an else at the end that assigns data_ some default value, which seems risky unless you have a suggestion, or I can just look at the types and promise the compiler data_ is going to get set. Thoughts?
There was a problem hiding this comment.
Interesting. I have never seen this before. We have a bunch of code in Firestore that we could simplify this way. So keep it as is please :)
| mappings.push(new Mapping('generation')); | ||
| mappings.push(new Mapping('metageneration')); | ||
| mappings.push(new Mapping('name', 'fullPath', true)); | ||
| const mappings: Array<Mapping<string> | Mapping<number>> = []; |
There was a problem hiding this comment.
Is this not just const mappings: Mappings?
| function validator(path: string): void { | ||
| if (/^[A-Za-z]+:\/\//.test(path)) { | ||
| function validator(path: unknown): void { | ||
| if (/^[A-Za-z]+:\/\//.test(path as string)) { |
There was a problem hiding this comment.
If you want to go the extra mile, then I would suggest we also validate explicitly that path is a string. Optional.
There was a problem hiding this comment.
Will give it a shot.
There was a problem hiding this comment.
Can you do the same thing for refFromURL below? Thanks!
Enable
"strict": trueTypescript flag for Storage.