Fix storage-types #3945
Fix storage-types #3945
Conversation
|
Binary Size ReportAffected SDKs
Test Logs
|
Size Analysis ReportAffected ProductsNo changes between base commit (0204abf) and head commit (9dd1d28). Test Logs
|
packages/storage-types/index.d.ts
Outdated
| | null | ||
| | ((a: UploadTaskSnapshot) => any), | ||
| error?: ((a: Error) => any) | null, | ||
| | ((a: UploadTaskSnapshot) => unknown), |
There was a problem hiding this comment.
This unknown seems inconsistent with the other return types.
Since this is going to land in a breaking change release, do you think it makes sense to start using more descriptive names than "a"? (here and everywhere)
There was a problem hiding this comment.
Sure, seems like a good idea.
packages/firebase/index.d.ts
Outdated
| name: string; | ||
| code: string; | ||
| message: string; | ||
| serverResponse: null | string; |
There was a problem hiding this comment.
Who adds serverResponse to the error object?
In #3946, I'm moving template data under a new field customData to avoid overwriting existing fields, should we do the same for serverResponse?
hsubox76
force-pushed
the
ch-storage-types
branch
from
8ec97ca
to
7e88943
Compare
|
Depends on #3946, rebase after that is merged. |
hsubox76
force-pushed
the
ch-storage-types
branch
from
7e88943
to
16c4ebe
Compare
packages/firebase/index.d.ts
Outdated
| * An error returned by the Firebase Storage SDK. | ||
| */ | ||
| interface FirebaseStorageError extends FirebaseError { | ||
| customData: { serverResponse: string | null }; |
There was a problem hiding this comment.
Since you have a getter serverResponse, customData feels like the internal implementation and you can just expose the getter.
Separated out from storage modularization PR: #3499
Should fix #1515