-
Notifications
You must be signed in to change notification settings - Fork 28.2k
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
srujzs
wants to merge
12
commits into
flutter:master
Choose a base branch
from
srujzs:migratepkgjs
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,740
−4,182
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Removes package:js and uses dart:js_interop instead This is taken directly from flutter#164254 and will be iterated on.
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.
kevmoo
reviewed
Mar 18, 2025
engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart
Show resolved
Hide resolved
@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. |
@srujzs – rebased to latest. Let's see how things look... |
Might could ponder our lint here. Even after these changes there are still 4 more lints...
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
@staticInterop
.historical relics due to not supporting primitives in externals.
Remove the redirecting functions and let the compiler handles the
conversions.
to all extension types where needed.
Pre-launch Checklist
///
).