Skip to content
This repository has been archived by the owner on May 30, 2023. It is now read-only.

feat(sdk): support latest SDK versions for Android and iOS #764

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

booleanbetrayal
Copy link

This PR updates SDKs for Android and iOS to the latest versions and provides override support as suggested by @marioshtika in the event that a non-conflicting / minor patch version can be targeted as needed. Several aspects of the iOS and Android code needed to be updated.

This unbreaks integration with other plugins such as @havesource/cordova-plugin-push.

NOTE: The current inception of this PR breaks support for trySilentLogin as it was not a required feature to support our app. I'm open to upstream contributions if it's something the community still needs, but it may be worth just shelving if the plugin is otherwise broken for the majority of users due to conflicting SDK dependencies across projects.

@@ -26,164 +26,131 @@ - (void)handleOpenURLWithAppSourceAndAnnotation:(NSNotification*)notification

if ([possibleReversedClientId isEqualToString:self.getreversedClientId] && self.isSigningIn) {
self.isSigningIn = NO;
[[GIDSignIn sharedInstance] handleURL:url];
[GIDSignIn.sharedInstance handleURL:url];
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see all breaking changes in v6.0.0 - https://developers.google.com/identity/sign-in/ios/release

@@ -194,7 +212,7 @@ private synchronized void buildGoogleApiClient(JSONObject clientOptions) throws
*/
private void signIn() {
Intent signInIntent = Auth.GoogleSignInApi.getSignInIntent(this.mGoogleApiClient);
cordova.getActivity().startActivityForResult(signInIntent, RC_GOOGLEPLUS);
this.signInActivityLauncher.launch(signInIntent);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines -9 to -14
- (void) isAvailable:(CDVInvokedUrlCommand*)command;
- (void) login:(CDVInvokedUrlCommand*)command;
- (void) trySilentLogin:(CDVInvokedUrlCommand*)command;
- (void) logout:(CDVInvokedUrlCommand*)command;
- (void) disconnect:(CDVInvokedUrlCommand*)command;
- (void) share_unused:(CDVInvokedUrlCommand*)command;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compatibility breakage in iOS:

  • isAvailable: even necessary?
  • trySilentLogin: open to backfills if community still requires it
  • share_unused: seemed to be cruft

@justinschier
Copy link

👍👍👍

@thewebkid
Copy link

I used this in a new cordova project (android only). I got it working by pulling the fork, merging the pr branch locally, and pointing cordova to my local dir. Somehow it actually just worked perfectly. Webstorm fussed about the bad git mappings, but I just ignored the warnings. Hope this helps someone else who is drowning in cordova auth hell like I was the past 2 weeks.

@marioshtika
Copy link

I think this pull request should be reviewed before merging it.
By removing the deprecated/unnecessary functionalities, this plugin will no longer be backward compatible.
I am already using the isAvailable and the trySilentLogin functions on my projects, so if I update it to this pr it will break my app.
Maybe those functionalities can be removed on a major release of the plugin example on version 9.0.0
Just food for thought :)

@booleanbetrayal
Copy link
Author

@marioshtika - I welcome any backfill for isAvailable and trySilentLogin, if people still require them, but they weren't necessary for us while the rest of these changes were. Wanted to get this PR up for posterity.

@jpike88
Copy link

jpike88 commented Dec 19, 2022

It's probably worth updating this PR as GoogleSignIn pod has now been updated to 7.0.0, it probably has breaking changes

@marioshtika
Copy link

This repository is not being updated in years, so any merge would be a success.

Let's hope @EddyVerbruggen hears our request for help 🤞

@boengli
Copy link

boengli commented Feb 28, 2023

Any update on this @EddyVerbruggen ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants