Gigya Web SDK integration - make example start properly on the web #45
Conversation
navaronbracke
changed the title
|
@tal-mi Since #41 has been merged (thanks for the amazing work getting this landed!), I updated this PR so we can get started with integrating the web SDK. Currently, Flutter 3.7.8 (the current stable release) does not allow |
lib/src/models/account.dart
Outdated
| this.verifiedTimestamp, | ||
| }); | ||
|
|
||
| /// Construct an account from the given [json]. | ||
| factory Account.fromJson(Map<String, dynamic> json) { | ||
| final Map<String, dynamic>? emails = json['emails'] == null ? null : json['emails'].cast<String, dynamic>() as Map<String, dynamic>; | ||
| final Map<String, dynamic>? profile = json['profile'] == null ? null : json['profile'].cast<String, dynamic>() as Map<String, dynamic>; | ||
| final Map<String, dynamic>? emails = json['emails'] as Map<String, dynamic>?; |
There was a problem hiding this comment.
@sagishm I noticed that you had changed to parsing of the emails / profile to handle the null case and casting to a Map.
Casting to a nullable Map, using as Map<String, dynamic>? should already handle both the null and Map<String, dynamic> cases (the ? handles nullable types). Also, using cast<K, V>() on a Map and then doing another cast using as, is redundant. So I restored the original code (from #41) here.
There was a problem hiding this comment.
Hi,
This change occurred because the code caused a parsing crash during our testing phase.
We believe it's a temporary Flutter version issue with LinkedHanshMap parsing.
We will keep it on hold until its clears and revert the change.
Thanks.
There was a problem hiding this comment.
@tal-mi I think I know what might be happening. The actual map is probably a Map<Object?, Object?>, which is what the binary messenger of the method channel returns. That type will indeed fail the forced cast with as, so in that case using cast<K,V>() is correct. I pushed a commit that should fix it for the account class. If there are other cases, we should fix that as we go I think?
For reference, I had to do the same thing with the screenset events stream.
There was a problem hiding this comment.
Yeah, think that might be it. We will fix it for sure.
We are currently going into the holidays so not many working days are available for the next two weeks.
One thing that is important. Because we are now on v1.0.0 of the plugin, breaking changes are a bit more difficult to approve due to how SAP handles customer notifications. Take that in mind for massive changes.
Of course, changes that are improving the plugin significantly will be prioritized.
Thanks again.
There was a problem hiding this comment.
I understand that any version after 1.0.0 needs to take care of breaking changes. Flutter's first party packages under flutter/packages follow a similar rule. The breaking changes in this PR should be relatively minor. I'll follow up with smaller PR's for the remaining web API's.
The web portion of the plugin will stay experimental until we can get all the major features from the mobile SDK implemented.
Enjoy the holidays!
|
@tal-mi If you have time to get this reviewed, we should be able to land experimental web support for users of Flutter 3.10 now. |
|
@tal-mi I tested all 4 methods that were implemented here and fixed the bugs I could find. (mostly null values, and one thing relating to Hot Reload) If you have the time to review this one, it would be greatly appreciated. |
navaronbracke
force-pushed
the
gigya_web_sdk
branch
from
4d3aeea to
f7f89d3
Compare
|
@tal-mi This initial PR to start on the Web SDK has gotten a little large, I'll split it up into smaller pieces for easier review. |
Gigya web sdk extension types for initSdk / isLoggedIn / login / logout
navaronbracke
force-pushed
the
gigya_web_sdk
branch
from
909c8a1 to
0dd5d37
Compare
navaronbracke
force-pushed
the
gigya_web_sdk
branch
from
a12f2c3 to
79fa12f
Compare
navaronbracke
force-pushed
the
gigya_web_sdk
branch
from
79fa12f to
4b7f3c3
Compare
This PR has been migrated to the new "extension types" syntax for JS static interop. This is a new language feature, that is still in development. This PR will be ready to review after that feature has shipped into stable.
This PR should be merged after #69 has been merged, as it builds upon it.
This PR implements the following methods for the web plugin implementation, so that the example app runs without errors at startup.
This PR includes a stub for the
InterruptionResolverFactory, to make the example app run on the web.This implementation still needs further implementing. But I will do that later.
The other two services,
OtpServiceandWebAuthenticationServicehave not yet been implemented.Related #44
Note: Due to using Dart's static Javascript interop, I needed to add a dependency on
js 0.6.6or higher, which requires Flutter 3.10 or higher. For that I had to bump the minimum Flutter version to 3.10