feat(datastore): Multi-auth #1478
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1478 +/- ##
=======================================
Coverage 46.47% 46.47%
=======================================
Files 261 262 +1
Lines 10191 10197 +6
=======================================
+ Hits 4736 4739 +3
- Misses 5455 5458 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| }) : super(token: token); | ||
|
|
||
| /// Internal use constructor | ||
| @protected | ||
| DataStorePluginInterface.tokenOnly({required Object token}) | ||
| : super(token: token); | ||
| : this(token: token, modelProvider: null); |
There was a problem hiding this comment.
What's the reason changing this line?
There was a problem hiding this comment.
It's identical to before, just saves me repeating myself again. Alternative is:
DataStorePluginInterface.tokenOnly({required Object token})
: authModeStrategy = AuthModeStrategy.default$,
super(token: token);| /// The raw value used for interfacing with native SDKs. | ||
| String get rawValue { | ||
| switch (this) { | ||
| case AuthModeStrategy.default$: |
There was a problem hiding this comment.
It looks like that the codebase mostly uses describeEnum as convention. But using describeEnum doesn't work with this escaped keyword...
Do we have option to name default as defaultAuth, or use uppercase like Android?
Or do developers need to use AuthModeStrategy.default at any occasion instead of passing null?
There was a problem hiding this comment.
Yeah, since default is a reserved word you can't use it as a member name. defaultAuth is fine, but it seemed redundant to say auth again.
Personally, I don't like using null in this context if there is a default value hidden behind it. Since it's a default value in the constructor, not passing any value is the same as passing AuthModeStrategy.default$, so the DX is the same except that the default value is apparent.
There was a problem hiding this comment.
I might make this AuthModeStrategy.defaultValue or AuthModeStrategy.defaultStrategy even though it is redundant.
My brain read this as AuthModeStrategy.$default and I was wondering if this was generated.
| /// The raw value used for interfacing with native SDKs. | ||
| String get rawValue { | ||
| switch (this) { | ||
| case AuthModeStrategy.default$: |
There was a problem hiding this comment.
I might make this AuthModeStrategy.defaultValue or AuthModeStrategy.defaultStrategy even though it is redundant.
My brain read this as AuthModeStrategy.$default and I was wondering if this was generated.
Issue #, if available:
#815
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.