Skip to content

fix(web): fix some casting issue on Web JS Interop #12852

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 30, 2024
Merged

fix(web): fix some casting issue on Web JS Interop #12852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
91fb37a
fix(web): fix some casting issue on Web JS Interop
Lyokone May 30, 2024
188ebed
format
Lyokone May 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion analysis_options.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ include: all_lint_rules.yaml
analyzer:
# TODO(rrousselGit): disable implicit-cast/implicit-dynamic
errors:
import_of_legacy_library_into_null_safe: ignore
# Otherwise cause the import of all_lint_rules to warn because of some rules conflicts.
# We explicitly enabled even conflicting rules and are fixing the conflict
# in this file
Expand Down Expand Up @@ -43,6 +42,8 @@ linter:
prefer_mixin: false
public_member_api_docs: false

invalid_runtime_check_with_js_interop_types: true

#############

# Far too verbose, and not that big of a deal when using parameter_assignments
Expand Down
4 changes: 4 additions & 0 deletions packages/cloud_firestore/cloud_firestore_web/lib/src/interop/firestore.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,14 @@ class LoadBundleTaskProgress
LoadBundleTaskProgress._fromJsObject(
firestore_interop.LoadBundleTaskProgressJsImpl jsObject,
) : taskState = convertToTaskState(jsObject.taskState.toDart.toLowerCase()),
// Cannot be done with Dart 3.2 constraints
// ignore: invalid_runtime_check_with_js_interop_types
bytesLoaded = jsObject.bytesLoaded is JSNumber
? (jsObject.bytesLoaded as JSNumber).toDartInt
: int.parse((jsObject.bytesLoaded as JSString).toDart),
documentsLoaded = jsObject.documentsLoaded.toDartInt,
// Cannot be done with Dart 3.2 constraints
// ignore: invalid_runtime_check_with_js_interop_types
totalBytes = jsObject.totalBytes is JSNumber
? (jsObject.totalBytes as JSNumber).toDartInt
: int.parse((jsObject.totalBytes as JSString).toDart),
Expand Down
7 changes: 6 additions & 1 deletion packages/cloud_firestore/cloud_firestore_web/lib/src/interop/utils/utils.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import '../firestore.dart';
/// Returns Dart representation from JS Object.
dynamic dartify(dynamic object) {
// Convert JSObject to Dart equivalents directly
// Cannot be done with Dart 3.2 constraints
// ignore: invalid_runtime_check_with_js_interop_types
if (object is! JSObject) {
return object;
}
Expand Down Expand Up @@ -85,11 +87,14 @@ JSAny? jsify(Object? dartObject) {
return jsifyFieldValue(dartObject);
}

// Cannot be done with Dart 3.2 constraints
// ignore: invalid_runtime_check_with_js_interop_types
if (dartObject is BytesJsImpl) {
return dartObject as JSAny;
}

// NOTE: if the firestore JS lib is not imported, we'll get a DDC warning here
// Cannot be done with Dart 3.2 constraints
// ignore: invalid_runtime_check_with_js_interop_types
if (dartObject is GeoPointJsImpl) {
return dartObject as JSAny;
}
Expand Down
4 changes: 4 additions & 0 deletions packages/cloud_firestore/cloud_firestore_web/lib/src/utils/decode_utility.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,16 @@ class DecodeUtility {
/// Decodes an incoming value to its proper type.
static dynamic valueDecode(
dynamic value, FirebaseFirestorePlatform firestore) {
// Cannot be done with Dart 3.2 constraints
// ignore: invalid_runtime_check_with_js_interop_types
if (value is JSObject &&
value.instanceof(GeoPointConstructor as JSFunction)) {
return GeoPoint((value as GeoPointJsImpl).latitude.toDartDouble,
(value as GeoPointJsImpl).longitude.toDartDouble);
} else if (value is DateTime) {
return Timestamp.fromDate(value);
// Cannot be done with Dart 3.2 constraints
// ignore: invalid_runtime_check_with_js_interop_types
Copy link
Collaborator

Choose a reason for hiding this comment

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

But this is invalid code, I think! Runtime behavior will be wrong w/ wasm!

Copy link

@srujzs srujzs May 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Drive-by comment:

It looks like both Dart values and JS values flow to value. In this case, I'd recommend checking all the Dart values first (is DateTime, etc.). After that, I'd cast value to a JSAny?, and then do isA checks with the resulting JSAny?. As an example:

if (value is DateTime) {
  ...
} else {
  // Note that if value is still a Dart value, this cast is unsafe, so check all the Dart values first.
  final jsValue = value as JSAny?;
  if (jsValue.isA<GeoPointJsImpl>()) {
    ...
  } else if (jsValue.isA<JSArray>()) {
    List<JSAny?> list = (jsValue as JSArray).toDart;
    ...
  }
}

It also looks like there's a check against List<dynamic>. If value here is actually a JS array, use isA<JSArray>, as is List<dynamic> may be true when compiling to JS, but false when compiling to Wasm. The general guidance is that if a value is a JS value, use interop to type-check it and not Dart is checks.

} else if (value is JSObject &&
value.instanceof(BytesConstructor as JSFunction)) {
return Blob((value as BytesJsImpl).toUint8Array().toDart);
Expand Down
2 changes: 2 additions & 0 deletions packages/cloud_functions/cloud_functions_web/lib/interop/functions.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ class HttpsCallable extends JsObjectWrapper<JSFunction> {
/// Returns Dart representation from JS Object.
dynamic _dartify(dynamic object) {
// Convert JSObject to Dart equivalents directly
// Cannot be done with Dart 3.2 constraints
// ignore: invalid_runtime_check_with_js_interop_types
if (object is! JSObject) {
return object;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/firebase_analytics/firebase_analytics_web/lib/interop/analytics.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class Analytics extends JsObjectWrapper<analytics_interop.AnalyticsJsImpl> {

static Future<bool> isSupported() async {
final result = await analytics_interop.isSupported().toDart;
return result! as bool;
return (result! as JSBoolean).toDart;
}

/// Non-null App for this instance of analytics service.
Expand Down
2 changes: 2 additions & 0 deletions packages/firebase_auth/firebase_auth_web/lib/firebase_auth_web.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,8 @@ class FirebaseAuthWeb extends FirebaseAuthPlatform {
.setItem(getOriginName(delegate.app.name), origin);
}
} catch (e) {
// Cannot be done with 3.2 constraints
// ignore: invalid_runtime_check_with_js_interop_types
if (e is auth_interop.AuthError) {
final String code = e.code.toDart;
// this catches Firebase Error from web that occurs after hot reloading & hot restarting
Expand Down
2 changes: 1 addition & 1 deletion ...firebase_auth/firebase_auth_web/lib/src/firebase_auth_web_recaptcha_verifier_factory.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class RecaptchaVerifierFactoryWeb extends RecaptchaVerifierFactoryPlatform {
@override
Future<int> render() async {
try {
return (await _delegate.render()).toInt();
return await _delegate.render();
} catch (e) {
throw getFirebaseAuthException(e);
}
Expand Down
14 changes: 7 additions & 7 deletions packages/firebase_auth/firebase_auth_web/lib/src/interop/auth.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ class User extends UserInfo<auth_interop.UserJsImpl> {
Future<String> getIdToken([bool forceRefresh = false]) => jsObject
.getIdToken(forceRefresh.toJS)
.toDart
.then((value) => value! as String);
.then((value) => (value! as JSString).toDart);

/// Links the user account with the given credentials, and returns any
/// available additional user information, such as user name.
Expand Down Expand Up @@ -526,7 +526,7 @@ class Auth extends JsObjectWrapper<auth_interop.AuthJsImpl> {
Future<List<String>> fetchSignInMethodsForEmail(String email) => auth_interop
.fetchSignInMethodsForEmail(jsObject, email.toJS)
.toDart
.then((value) => List<String>.from(value! as List<dynamic>));
.then((value) => List<String>.from((value! as JSArray).toDart));

/// Checks if an incoming link is a sign-in with email link.
bool isSignInWithEmailLink(String emailLink) =>
Expand Down Expand Up @@ -753,7 +753,7 @@ class Auth extends JsObjectWrapper<auth_interop.AuthJsImpl> {
Future<String> verifyPasswordResetCode(String code) => auth_interop
.verifyPasswordResetCode(jsObject, code.toJS)
.toDart
.then((value) => value! as String);
.then((value) => (value! as JSString).toDart);
}

/// Represents an auth provider.
Expand Down Expand Up @@ -1054,7 +1054,7 @@ class PhoneAuthProvider
jsObject
.verifyPhoneNumber(phoneOptions, applicationVerifier.jsObject)
.toDart
.then((value) => value! as String);
.then((value) => (value! as JSString).toDart);

/// Creates a phone auth credential given the verification ID
/// from [verifyPhoneNumber] and the [verificationCode] that was sent to the
Expand All @@ -1081,7 +1081,7 @@ abstract class ApplicationVerifier<
/// Returns a Future containing string for a token that can be used to
/// assert the validity of a request.
Future<String> verify() =>
jsObject.verify().toDart.then((value) => value! as String);
jsObject.verify().toDart.then((value) => (value! as JSString).toDart);
}

/// reCAPTCHA verifier.
Expand Down Expand Up @@ -1137,8 +1137,8 @@ class RecaptchaVerifier

/// Renders the reCAPTCHA widget on the page.
/// Returns a Future that resolves with the reCAPTCHA widget ID.
Future<num> render() =>
jsObject.render().toDart.then((value) => value! as num);
Future<int> render() =>
jsObject.render().toDart.then((value) => (value! as JSNumber).toDartInt);
}

/// A result from a phone number sign-in, link, or reauthenticate call.
Expand Down
3 changes: 3 additions & 0 deletions packages/firebase_core/firebase_core_web/lib/src/firebase_core_web.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@ class FirebaseCoreWeb extends FirebasePlatform {
JSObject? ignored =
globalContext.getProperty('flutterfire_ignore_scripts'.toJS);

// Cannot be done with Dart 3.2 constraints
// ignore: invalid_runtime_check_with_js_interop_types
if (ignored is Iterable) {
// ignore: invalid_runtime_check_with_js_interop_types
return (ignored! as Iterable)
.map((e) => e.toString())
.toList(growable: false);
Expand Down
2 changes: 1 addition & 1 deletion packages/firebase_messaging/firebase_messaging_web/lib/firebase_messaging_web.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class FirebaseMessagingWeb extends FirebaseMessagingPlatform {
}) {
return convertWebExceptions(() async {
String status =
(await web.Notification.requestPermission().toDart) as String;
(await web.Notification.requestPermission().toDart).toDart;
return utils.getNotificationSettings(status);
});
}
Expand Down
22 changes: 13 additions & 9 deletions packages/firebase_messaging/firebase_messaging_web/lib/src/interop/messaging.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ class Messaging extends JsObjectWrapper<messaging_interop.MessagingJsImpl> {
return _expando[jsObject] ??= Messaging._fromJsObject(jsObject);
}

static Future<bool> isSupported() =>
messaging_interop.isSupported().toDart.then((value) => value! as bool);
static Future<bool> isSupported() => messaging_interop
.isSupported()
.toDart
.then((value) => (value! as JSBoolean).toDart);

Messaging._fromJsObject(messaging_interop.MessagingJsImpl jsObject)
: super.fromJsObject(jsObject);
Expand All @@ -45,13 +47,15 @@ class Messaging extends JsObjectWrapper<messaging_interop.MessagingJsImpl> {
/// that can be used to send push messages to this user.
Future<String> getToken({String? vapidKey}) async {
try {
final token = (await messaging_interop
.getToken(
jsObject,
vapidKey == null
? null
: messaging_interop.GetTokenOptions(vapidKey: vapidKey.toJS))
.toDart)! as String;
final token = ((await messaging_interop
.getToken(
jsObject,
vapidKey == null
? null
: messaging_interop.GetTokenOptions(
vapidKey: vapidKey.toJS))
.toDart)! as JSString)
.toDart;
return token;
} catch (err) {
// A race condition can happen in which the service worker get registered
Expand Down
4 changes: 2 additions & 2 deletions ...base_remote_config/firebase_remote_config_web/lib/src/interop/firebase_remote_config.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class RemoteConfig
Future<bool> activate() async => remote_config_interop
.activate(jsObject)
.toDart
.then((value) => value! as bool);
.then((value) => (value! as JSBoolean).toDart);

/// Ensures the last activated config are available to the getters.
Future<void> ensureInitialized() async =>
Expand All @@ -101,7 +101,7 @@ class RemoteConfig
/// If the fetched configs were already activated, the promise will resolve to false.
Future<bool> fetchAndActivate() async =>
remote_config_interop.fetchAndActivate(jsObject).toDart.then(
(value) => value! as bool,
(value) => (value! as JSBoolean).toDart,
);

/// Returns all config values.
Expand Down
Loading