Accept Compat types in firestore-exp API #4056
Conversation
| @@ -166,7 +166,8 @@ export function tryGetCustomObjectType(input: object): string | null { | |||
| } | |||
|
|
|||
| /** | |||
| * Casts `obj` to `T`. Throws if `obj` is not an instance of `T`. | |||
| * Casts `obj` to `T`, optionally unwrapping Compat types to expose the | |||
There was a problem hiding this comment.
Can we just check obj._delegate instead of relying on instanceofing the abstract class, so that I can put it into @firebase/util and share with other SDKs?
There was a problem hiding this comment.
Updated. This function also does a name check and provides a different error message if there it detects that two classes share the same name but are not compatible. I was wondering if we need to strip suffixes here, since Rollup adds "$1" and counting during the build phase?
There was a problem hiding this comment.
Thanks. Yes, rollup will append $x to things with the same name if they end up in the same bundle. I'm not sure if it's the case with webpack which creates many closures instead of one big top level scope. I think it very much depends on the format of the bundle, thus is hard for us to predict/optimize for. I would just leave it as is for now and see if developer reports that the error is not helpful.
|
To answer your question -I am envisioning a world where all types are fully exchangeable. The user will not need to do any casting. |
|
@Feiyang1 Can you take another look? |
| @@ -166,7 +166,8 @@ export function tryGetCustomObjectType(input: object): string | null { | |||
| } | |||
|
|
|||
| /** | |||
| * Casts `obj` to `T`. Throws if `obj` is not an instance of `T`. | |||
| * Casts `obj` to `T`, optionally unwrapping Compat types to expose the | |||
There was a problem hiding this comment.
Thanks. Yes, rollup will append $x to things with the same name if they end up in the same bundle. I'm not sure if it's the case with webpack which creates many closures instead of one big top level scope. I think it very much depends on the format of the bundle, thus is hard for us to predict/optimize for. I would just leave it as is for now and see if developer reports that the error is not helpful.
|
This PR allows all firestore-exp APIs to transparently accept Compat types, which makes migration for the user easier.