Skip to content
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.

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

[WIP] [web] Remove package:js in favor of dart:js_interop #165324

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

srujzs
Copy link
Contributor

@srujzs srujzs commented Mar 17, 2025

Removes package:js and uses dart:js_interop instead

This is taken directly from #164254 and will be iterated on.


Initial commit is the original PR % rebasing. Follow-up commits address test/analysis failures and commits after those clean up JS interop code:

  • Remove remaining uses of @staticInterop.
  • Code that uses JSBoolean, JSString, or JSNumber are mostly
    historical relics due to not supporting primitives in externals.
    Remove the redirecting functions and let the compiler handles the
    conversions.
  • Replace domInstanceOfString calls with isA and add @js annotations
    to all extension types where needed.
  • Remove unnecessary @js annotations.
  • Remove unneeded implements JSObject clauses.
  • Standardize naming of extension types in some cases.
  • Remove JSVoid in favor of void.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
  • All existing and new tests are passing.

@github-actions github-actions bot added a: text input Entering text in a text field or keyboard related problems engine flutter/engine repository. See also e: labels. a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: scrolling Viewports, list views, slivers, etc. f: routes Navigator, Router, and related APIs. platform-web Web applications specifically f: focus Focus traversal, gaining or losing focus labels Mar 17, 2025
@srujzs srujzs changed the title [web] Remove package:js in favor of dart:js_interop [WIP] [web] Remove package:js in favor of dart:js_interop Mar 17, 2025
srujzs added 2 commits March 17, 2025 11:21
Removes package:js and uses dart:js_interop instead

This is taken directly from flutter#164254 and
will be iterated on.
srujzs added 9 commits March 17, 2025 14:38
It looks like the builders don't like this annotation. Instead, remove
the extraneous definitions.
- Code that uses JSBoolean, JSString, or JSNumber are mostly
historical relics due to not supporting primitives in externals.
Remove the redirecting functions and let the compiler handles the
conversions.
- Replace domInstanceOfString calls with isA and add @js annotations
to all extension types where needed.
- Remove unnecessary @js annotations.
- Remove unneeded implements JSObject clauses.
- Standardize naming of extension types in some cases.
@srujzs
Copy link
Contributor Author

srujzs commented Mar 19, 2025

@kevmoo FYI I'll be out for a little while but will come back to this and review test failures, but please feel free to review in the interim.

@kevmoo
Copy link
Contributor

kevmoo commented Mar 20, 2025

@srujzs – rebased to latest. Let's see how things look...

@kevmoo
Copy link
Contributor

kevmoo commented Mar 20, 2025

Might could ponder our lint here. Even after these changes there are still 4 more lints...

info: Cast from 'T' to 'SkDeletable' casts a Dart value to a JS interop type, which might not be platform-consistent. (invalid_runtime_check_with_js_interop_types at [ui] lib/src/engine/canvaskit/native_memory.dart:85)
info: Cast from 'JSUint8Array' to 'T' casts a JS interop value to an incompatible JS interop type, which might not be platform-consistent. (invalid_runtime_check_with_js_interop_types at [ui] lib/src/engine/dom.dart:1274)
info: Runtime check between 'Object' and 'JSAny' checks whether a Dart value is a JS interop type, which might not be platform-consistent. (invalid_runtime_check_with_js_interop_types at [ui] lib/src/engine/image_decoder.dart:152)
info: Runtime check between 'Object?' and 'DomElement' checks whether a Dart value is a JS interop type, which might not be platform-consistent. (invalid_runtime_check_with_js_interop_types at [ui] test/common/matchers.dart:262)
diff --git a/engine/src/flutter/lib/web_ui/analysis_options.yaml b/engine/src/flutter/lib/web_ui/analysis_options.yaml
index 3a172dcb3c..b42eeac6e0 100644
--- a/engine/src/flutter/lib/web_ui/analysis_options.yaml
+++ b/engine/src/flutter/lib/web_ui/analysis_options.yaml
@@ -10,7 +10,7 @@ linter:
   rules:
     avoid_print: false
     avoid_setters_without_getters: false
-    invalid_runtime_check_with_js_interop_types: false
+    #invalid_runtime_check_with_js_interop_types: false
     library_private_types_in_public_api: false
     no_default_cases: false
     prefer_relative_imports: false
diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart
index 122e24dc63..f9e3fbf932 100644
--- a/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart
+++ b/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart
@@ -1206,7 +1206,7 @@ abstract class HttpFetchPayload {
   ///
   /// Combined with [HttpFetchResponse.contentLength], this can be used to
   /// implement various "progress bar" functionality.
-  Future<void> read<T>(HttpFetchReader<T> reader);
+  Future<void> read<T extends JSAny>(HttpFetchReader<T> reader);
 
   /// Returns the data as a [ByteBuffer].
   Future<ByteBuffer> asByteBuffer();
@@ -1224,7 +1224,7 @@ class HttpFetchPayloadImpl implements HttpFetchPayload {
   final DomResponse _domResponse;
 
   @override
-  Future<void> read<T>(HttpFetchReader<T> callback) async {
+  Future<void> read<T extends JSAny>(HttpFetchReader<T> callback) async {
     final _DomReadableStream stream = _domResponse.body;
     final _DomStreamReader reader = stream.getReader();
 
@@ -1233,7 +1233,7 @@ class HttpFetchPayloadImpl implements HttpFetchPayload {
       if (chunk.done) {
         break;
       }
-      callback(chunk.value as T);
+      callback(chunk.value! as T);
     }
   }
 
@@ -1261,7 +1261,7 @@ class MockHttpFetchPayload implements HttpFetchPayload {
   final int _chunkSize;
 
   @override
-  Future<void> read<T>(HttpFetchReader<T> callback) async {
+  Future<void> read<T extends JSAny>(HttpFetchReader<T> callback) async {
     final int totalLength = _byteBuffer.lengthInBytes;
     int currentIndex = 0;
     while (currentIndex < totalLength) {
@@ -2267,7 +2267,7 @@ extension type _DomList._(JSObject _) implements JSObject {
   external DomNode item(int index);
 }
 
-class _DomListIterator<T> implements Iterator<T> {
+class _DomListIterator<T extends JSObject> implements Iterator<T> {
   _DomListIterator(this.list);
 
   final _DomList list;
@@ -2286,7 +2286,7 @@ class _DomListIterator<T> implements Iterator<T> {
   T get current => list.item(index) as T;
 }
 
-class _DomListWrapper<T> extends Iterable<T> {
+class _DomListWrapper<T extends JSObject> extends Iterable<T> {
   _DomListWrapper._(this.list);
 
   final _DomList list;
@@ -2301,15 +2301,16 @@ class _DomListWrapper<T> extends Iterable<T> {
 
 /// This is a work around for a `TypeError` which can be triggered by calling
 /// `toList` on the `Iterable`.
-Iterable<T> createDomListWrapper<T>(_DomList list) => _DomListWrapper<T>._(list).cast<T>();
+Iterable<T> createDomListWrapper<T extends JSObject>(_DomList list) =>
+    _DomListWrapper<T>._(list).cast<T>();
 
 // https://developer.mozilla.org/en-US/docs/Web/API/TouchList
 extension type _DomTouchList._(JSObject _) implements JSObject {
   external double get length;
-  external DomNode item(int index);
+  external DomTouch item(int index);
 }
 
-class _DomTouchListIterator<T> implements Iterator<T> {
+class _DomTouchListIterator<T extends DomTouch> implements Iterator<T> {
   _DomTouchListIterator(this.list);
 
   final _DomTouchList list;
@@ -2328,7 +2329,7 @@ class _DomTouchListIterator<T> implements Iterator<T> {
   T get current => list.item(index) as T;
 }
 
-class _DomTouchListWrapper<T> extends Iterable<T> {
+class _DomTouchListWrapper<T extends DomTouch> extends Iterable<T> {
   _DomTouchListWrapper._(this.list);
 
   final _DomTouchList list;
@@ -2341,7 +2342,7 @@ class _DomTouchListWrapper<T> extends Iterable<T> {
   int get length => list.length.toInt();
 }
 
-Iterable<T> createDomTouchListWrapper<T>(_DomTouchList list) =>
+Iterable<T> createDomTouchListWrapper<T extends DomTouch>(_DomTouchList list) =>
     _DomTouchListWrapper<T>._(list).cast<T>();
 
 @JS('Symbol')
@@ -2394,7 +2395,7 @@ extension type DomIteratorResult._(JSObject _) implements JSObject {
 }
 
 /// Wraps a native JS iterator to provide a Dart [Iterator].
-class DomIteratorWrapper<T> implements Iterator<T> {
+class DomIteratorWrapper<T extends JSAny> implements Iterator<T> {
   DomIteratorWrapper(this._iterator);
 
   final DomIterator _iterator;
diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart
index fb74b6da9e..a7d73c1929 100644
--- a/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart
+++ b/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'dart:async';
+import 'dart:js_interop';
 
 import 'package:meta/meta.dart';
 import 'package:ui/src/engine.dart';
@@ -86,7 +87,7 @@ final class ViewFocusBinding {
     // The right event type needs to be checked because Chrome seems to be firing
     // `Event` events instead of `KeyboardEvent` events when autofilling is used.
     // See https://github.com/flutter/flutter/issues/149968 for more info.
-    if (event is DomKeyboardEvent && (event.shiftKey ?? false)) {
+    if (event.isA<DomKeyboardEvent>() && ((event as DomKeyboardEvent).shiftKey ?? false)) {
       _viewFocusDirection = ui.ViewFocusDirection.backward;
     }
   });
diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/pointer_binding.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/pointer_binding.dart
index 2552a3af5d..2c60bde1eb 100644
--- a/engine/src/flutter/lib/web_ui/lib/src/engine/pointer_binding.dart
+++ b/engine/src/flutter/lib/web_ui/lib/src/engine/pointer_binding.dart
@@ -322,7 +322,9 @@ class ClickDebouncer {
     assert(event.type == 'pointerdown', 'Click debouncing must begin with a pointerdown');
 
     final DomEventTarget? target = event.target;
-    if (target is DomElement && target.hasAttribute('flt-tappable')) {
+    if (target != null &&
+        (target.isA<DomElement>()) &&
+        (target as DomElement).hasAttribute('flt-tappable')) {
       _state = (
         target: target,
         // The 200ms duration was chosen empirically by testing tapping, mouse

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: text input Entering text in a text field or keyboard related problems engine flutter/engine repository. See also e: labels. f: focus Focus traversal, gaining or losing focus f: routes Navigator, Router, and related APIs. f: scrolling Viewports, list views, slivers, etc. platform-web Web applications specifically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants