Skip to content

Commit

Permalink
[google_sign_in/google_identity_services] Clear-up documentation of c…
Browse files Browse the repository at this point in the history
…allbacks in various APIs and uses of those APIs (flutter#8029)

These APIs used to accept allowInterop'd callbacks. When the
implementation was migrated to use dart:js_interop, they were wrapped
with toJS. However, starting in 3.5, to be consistent with dart2wasm,
double-wrapping a function is an error. The documentation should be
clear that it accepts only Dart functions and uses should be cleaned up
to not use allowInterop and type the callbacks' parameter and return
value correctly.
  • Loading branch information
srujzs authored Nov 7, 2024
1 parent da51281 commit 4b2132f
Show file tree
Hide file tree
Showing 9 changed files with 25 additions and 23 deletions.
5 changes: 4 additions & 1 deletion packages/google_identity_services_web/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
## NEXT
## 0.3.1+5

* Updates minimum supported SDK version to Flutter 3.22/Dart 3.4.
* Cleans up documentation of callbacks in `CodeClientConfig`,
`TokenClientConfig`, `onGoogleLibraryLoad`, and `revoke` to indicate they only
accept Dart functions and not JS functions.

## 0.3.1+4

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ void main() async {
testWidgets('TokenClientConfig', (_) async {
final TokenClientConfig config = TokenClientConfig(
client_id: 'testing_1-2-3',
callback: (_) {},
callback: (TokenResponse _) {},
scope: <String>['one', 'two', 'three'],
include_granted_scopes: true,
prompt: 'some-prompt',
enable_granular_consent: true,
login_hint: '[email protected]',
hd: 'hd_value',
state: 'some-state',
error_callback: (_) {},
error_callback: (GoogleIdentityServicesError? _) {},
);

final utils.ExpectConfigValueFn expectConfigValue =
Expand Down Expand Up @@ -79,14 +79,14 @@ void main() async {
scope: <String>['one', 'two', 'three'],
include_granted_scopes: true,
redirect_uri: Uri.parse('https://www.example.com/login'),
callback: (_) {},
callback: (CodeResponse _) {},
state: 'some-state',
enable_granular_consent: true,
login_hint: '[email protected]',
hd: 'hd_value',
ux_mode: UxMode.popup,
select_account: true,
error_callback: (_) {},
error_callback: (GoogleIdentityServicesError? _) {},
);

final utils.ExpectConfigValueFn expectConfigValue =
Expand All @@ -110,7 +110,7 @@ void main() async {
testWidgets('returns a tokenClient', (_) async {
final TokenClient client = oauth2.initTokenClient(TokenClientConfig(
client_id: 'for-tests',
callback: (_) {},
callback: (TokenResponse _) {},
scope: <String>['some_scope', 'for_tests', 'not_real'],
));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ extension GoogleAccountsIdExtension on GoogleAccountsId {
/// ID is the `sub` property of the [CredentialResponse.credential] payload.
///
/// The optional [callback] is a function that gets called to report on the
/// success of the revocation call.
/// success of the revocation call. It must be a Dart function and not a JS
/// function.
///
/// Method: google.accounts.id.revoke
/// https://developers.google.com/identity/gsi/web/reference/js-reference#google.accounts.id.revoke
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ extension GoogleAccountsOauth2Extension on GoogleAccountsOauth2 {
/// A valid [accessToken] is required to revoke permissions.
///
/// The [done] callback is called once the revoke action is done. It must be
/// manually wrapped in [allowInterop] before being passed to this method.
/// a Dart function and not a JS function.
///
/// Method: google.accounts.oauth2.revoke
/// https://developers.google.com/identity/oauth2/web/reference/js-reference#google.accounts.oauth2.revoke
Expand Down Expand Up @@ -103,8 +103,7 @@ extension GoogleAccountsOauth2Extension on GoogleAccountsOauth2 {
abstract class CodeClientConfig {
/// Constructs a CodeClientConfig object in JavaScript.
///
/// The [callback] property must be wrapped in [allowInterop] before it's
/// passed to this constructor.
/// The [callback] property must be a Dart function and not a JS function.
factory CodeClientConfig({
required String client_id,
required List<String> scope,
Expand Down Expand Up @@ -230,8 +229,7 @@ typedef CodeClientCallbackFn = void Function(CodeResponse response);
abstract class TokenClientConfig {
/// Constructs a TokenClientConfig object in JavaScript.
///
/// The [callback] property must be wrapped in [allowInterop] before it's
/// passed to this constructor.
/// The [callback] property must be a Dart function and not a JS function.
factory TokenClientConfig({
required String client_id,
required TokenClientCallbackFn callback,
Expand Down Expand Up @@ -315,9 +313,6 @@ extension TokenClientExtension on TokenClient {
@staticInterop
abstract class OverridableTokenClientConfig {
/// Constructs an OverridableTokenClientConfig object in JavaScript.
///
/// The [callback] property must be wrapped in [allowInterop] before it's
/// passed to this constructor.
factory OverridableTokenClientConfig({
/// A list of scopes that identify the resources that your application could
/// access on the user's behalf. These values inform the consent screen that
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ external set _onGoogleLibraryLoad(JSFunction callback);

/// Method called after the Sign In With Google JavaScript library is loaded.
///
/// The [callback] parameter must be manually wrapped in [allowInterop]
/// before being set to the [onGoogleLibraryLoad] property.
/// The [function] parameter must be a Dart function and not a JS function.
set onGoogleLibraryLoad(VoidFn function) {
_onGoogleLibraryLoad = function.toJS;
}
2 changes: 1 addition & 1 deletion packages/google_identity_services_web/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: google_identity_services_web
description: A Dart JS-interop layer for Google Identity Services. Google's new sign-in SDK for Web that supports multiple types of credentials.
repository: https://github.com/flutter/packages/tree/main/packages/google_identity_services_web
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+google_identiy_services_web%22
version: 0.3.1+4
version: 0.3.1+5

environment:
sdk: ^3.4.0
Expand Down
4 changes: 4 additions & 0 deletions packages/google_sign_in/google_sign_in_web/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.12.4+3

* Fixes callback types for `TokenClientConfig`.

## 0.12.4+2

* Adds support for `web: ^1.0.0`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,9 @@ class GisSdkClient {
//
// Token clients have an additional `error_callback` for miscellaneous
// errors, like "popup couldn't open" or "popup closed by user".
void _onTokenError(Object? error) {
void _onTokenError(GoogleIdentityServicesError? error) {
if (error != null) {
_tokenResponses.addError((error as GoogleIdentityServicesError).type);
_tokenResponses.addError(error.type);
}
}

Expand Down Expand Up @@ -223,9 +223,9 @@ class GisSdkClient {
}
}

void _onCodeError(Object? error) {
void _onCodeError(GoogleIdentityServicesError? error) {
if (error != null) {
_codeResponses.addError((error as GoogleIdentityServicesError).type);
_codeResponses.addError(error.type);
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/google_sign_in/google_sign_in_web/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ description: Flutter plugin for Google Sign-In, a secure authentication system
for signing in with a Google account on Android, iOS and Web.
repository: https://github.com/flutter/packages/tree/main/packages/google_sign_in/google_sign_in_web
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+google_sign_in%22
version: 0.12.4+2
version: 0.12.4+3

environment:
sdk: ^3.3.0
Expand Down

0 comments on commit 4b2132f

Please sign in to comment.