Skip to content

Commit c1cb617

Browse files
authored
[web] Avoid returning List from DOM apis. (flutter#33880)
1 parent 448d16e commit c1cb617

File tree

4 files changed

+74
-24
lines changed

4 files changed

+74
-24
lines changed

lib/web_ui/lib/src/engine/dom.dart

Lines changed: 66 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,9 @@ class DomDocument {}
6666
extension DomDocumentExtension on DomDocument {
6767
external DomElement? get documentElement;
6868
external DomElement? querySelector(String selectors);
69-
List<DomElement> querySelectorAll(String selectors) =>
70-
js_util.callMethod<List<Object?>>(
71-
this, 'querySelectorAll', <Object>[selectors]).cast<DomElement>();
69+
Iterable<DomElement> querySelectorAll(String selectors) =>
70+
_DomElementListWrapper.create(js_util.callMethod<_DomElementList>(
71+
this, 'querySelectorAll', <Object>[selectors]));
7272
DomElement createElement(String name, [Object? options]) =>
7373
js_util.callMethod(this, 'createElement',
7474
<Object>[name, if (options != null) options]) as DomElement;
@@ -172,8 +172,8 @@ class DomElement extends DomNode {}
172172
DomElement createDomElement(String tag) => domDocument.createElement(tag);
173173

174174
extension DomElementExtension on DomElement {
175-
List<DomElement> get children =>
176-
js_util.getProperty<List<Object?>>(this, 'children').cast<DomElement>();
175+
Iterable<DomElement> get children => _DomElementListWrapper.create(
176+
js_util.getProperty<_DomElementList>(this, 'children'));
177177
external int get clientHeight;
178178
external int get clientWidth;
179179
external String get id;
@@ -188,9 +188,9 @@ extension DomElementExtension on DomElement {
188188
external DomRect getBoundingClientRect();
189189
external void prepend(DomNode node);
190190
external DomElement? querySelector(String selectors);
191-
List<DomElement> querySelectorAll(String selectors) =>
192-
js_util.callMethod<List<Object?>>(
193-
this, 'querySelectorAll', <Object>[selectors]).cast<DomElement>();
191+
Iterable<DomElement> querySelectorAll(String selectors) =>
192+
_DomElementListWrapper.create(js_util.callMethod<_DomElementList>(
193+
this, 'querySelectorAll', <Object>[selectors]));
194194
external void remove();
195195
external void setAttribute(String name, Object value);
196196
void appendText(String text) => append(createDomText(text));
@@ -207,16 +207,14 @@ extension DomCSSStyleDeclarationExtension on DomCSSStyleDeclaration {
207207
set clip(String value) => setProperty('clip', value);
208208
set clipPath(String value) => setProperty('clip-path', value);
209209
set transform(String value) => setProperty('transform', value);
210-
set transformOrigin(String value) =>
211-
setProperty('transform-origin', value);
210+
set transformOrigin(String value) => setProperty('transform-origin', value);
212211
set opacity(String value) => setProperty('opacity', value);
213212
set color(String value) => setProperty('color', value);
214213
set top(String value) => setProperty('top', value);
215214
set left(String value) => setProperty('left', value);
216215
set right(String value) => setProperty('right', value);
217216
set bottom(String value) => setProperty('bottom', value);
218-
set backgroundColor(String value) =>
219-
setProperty('background-color', value);
217+
set backgroundColor(String value) => setProperty('background-color', value);
220218
set pointerEvents(String value) => setProperty('pointer-events', value);
221219
set filter(String value) => setProperty('filter', value);
222220
set zIndex(String value) => setProperty('z-index', value);
@@ -251,8 +249,7 @@ extension DomCSSStyleDeclarationExtension on DomCSSStyleDeclaration {
251249
set borderRadius(String value) => setProperty('border-radius', value);
252250
set perspective(String value) => setProperty('perspective', value);
253251
set padding(String value) => setProperty('padding', value);
254-
set backgroundImage(String value) =>
255-
setProperty('background-image', value);
252+
set backgroundImage(String value) => setProperty('background-image', value);
256253
set border(String value) => setProperty('border', value);
257254
set mixBlendMode(String value) => setProperty('mix-blend-mode', value);
258255
set backgroundSize(String value) => setProperty('background-size', value);
@@ -655,11 +652,11 @@ extension DomHTMLTextAreaElementExtension on DomHTMLTextAreaElement {
655652
class DomClipboard extends DomEventTarget {}
656653

657654
extension DomClipboardExtension on DomClipboard {
658-
Future<String> readText() =>
659-
js_util.promiseToFuture<String>(js_util.callMethod(this, 'readText', <Object>[]));
655+
Future<String> readText() => js_util.promiseToFuture<String>(
656+
js_util.callMethod(this, 'readText', <Object>[]));
660657

661-
Future<dynamic> writeText(String data) =>
662-
js_util.promiseToFuture(js_util.callMethod(this, 'readText', <Object>[data]));
658+
Future<dynamic> writeText(String data) => js_util
659+
.promiseToFuture(js_util.callMethod(this, 'readText', <Object>[data]));
663660
}
664661

665662
extension DomResponseExtension on DomResponse {
@@ -694,6 +691,57 @@ extension DomKeyboardEventExtension on DomKeyboardEvent {
694691
external bool getModifierState(String keyArg);
695692
}
696693

694+
/// [_DomElementList] is the shared interface for APIs that return either
695+
/// `NodeList` or `HTMLCollection`. Do *not* add any API to this class that
696+
/// isn't support by both JS objects. Furthermore, this is an internal class and
697+
/// should only be returned as a wrapped object to Dart.
698+
@JS()
699+
@staticInterop
700+
class _DomElementList {}
701+
702+
extension DomElementListExtension on _DomElementList {
703+
external int get length;
704+
DomElement item(int index) =>
705+
js_util.callMethod<DomElement>(this, 'item', <Object>[index]);
706+
}
707+
708+
class _DomElementListIterator extends Iterator<DomElement> {
709+
final _DomElementList elementList;
710+
int index = -1;
711+
712+
_DomElementListIterator(this.elementList);
713+
714+
@override
715+
bool moveNext() {
716+
index++;
717+
if (index > elementList.length) {
718+
throw 'Iterator out of bounds';
719+
}
720+
return index < elementList.length;
721+
}
722+
723+
@override
724+
DomElement get current => elementList.item(index);
725+
}
726+
727+
class _DomElementListWrapper extends Iterable<DomElement> {
728+
final _DomElementList elementList;
729+
730+
_DomElementListWrapper._(this.elementList);
731+
732+
/// This is a work around for a `TypeError` which can be triggered by calling
733+
/// `toList` on the `Iterable`.
734+
static Iterable<DomElement> create(_DomElementList elementList) =>
735+
_DomElementListWrapper._(elementList).cast<DomElement>();
736+
737+
@override
738+
Iterator<DomElement> get iterator => _DomElementListIterator(elementList);
739+
740+
/// Override the length to avoid iterating through the whole collection.
741+
@override
742+
int get length => elementList.length;
743+
}
744+
697745
Object? domGetConstructor(String constructorName) =>
698746
js_util.getProperty(domWindow, constructorName);
699747

lib/web_ui/lib/src/engine/html/surface_stats.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ void debugPrintSurfaceStats(PersistedScene scene, int frameNumber) {
291291
// A microtask will fire after the DOM is flushed, letting us probe into
292292
// actual <canvas> tags.
293293
scheduleMicrotask(() {
294-
final List<DomElement> canvasElements = domDocument.querySelectorAll('canvas');
294+
final Iterable<DomElement> canvasElements = domDocument.querySelectorAll('canvas');
295295
final StringBuffer canvasInfo = StringBuffer();
296296
final int pixelCount = canvasElements
297297
.cast<DomCanvasElement>()

lib/web_ui/test/engine/surface/scene_builder_test.dart

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ void testMain() {
248248

249249
final DomElement contentAfterReuse = builder2.build().webOnlyRootElement!;
250250
final List<DomCanvasElement> list =
251-
contentAfterReuse.querySelectorAll('canvas').cast<DomCanvasElement>();
251+
contentAfterReuse.querySelectorAll('canvas').cast<DomCanvasElement>().toList();
252252
expect(list[0].style.zIndex, '-1');
253253
expect(list[1].style.zIndex, '');
254254
});
@@ -268,7 +268,7 @@ void testMain() {
268268
final DomElement content = builder.build().webOnlyRootElement!;
269269
domDocument.body!.append(content);
270270
List<DomHTMLImageElement> list =
271-
content.querySelectorAll('img').cast<DomHTMLImageElement>();
271+
content.querySelectorAll('img').cast<DomHTMLImageElement>().toList();
272272
for (final DomHTMLImageElement image in list) {
273273
image.alt = 'marked';
274274
}
@@ -284,7 +284,8 @@ void testMain() {
284284
builder2.pop();
285285

286286
final DomElement contentAfterReuse = builder2.build().webOnlyRootElement!;
287-
list = contentAfterReuse.querySelectorAll('img').cast<DomHTMLImageElement>();
287+
list =
288+
contentAfterReuse.querySelectorAll('img').cast<DomHTMLImageElement>().toList();
288289
for (final DomHTMLImageElement image in list) {
289290
expect(image.alt, 'marked');
290291
}
@@ -515,7 +516,8 @@ void testMain() {
515516
renderedLayers[char] = pushChild(builder, char, oldLayer: renderedLayers[char]);
516517
}
517518
final SurfaceScene scene = builder.build();
518-
final List<DomElement> pTags = scene.webOnlyRootElement!.querySelectorAll('flt-paragraph');
519+
final List<DomElement> pTags =
520+
scene.webOnlyRootElement!.querySelectorAll('flt-paragraph').toList();
519521
expect(pTags, hasLength(string.length));
520522
expect(
521523
scene.webOnlyRootElement!.querySelectorAll('flt-paragraph').map((DomElement p) => p.innerText).join(''),

lib/web_ui/test/text_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ Future<void> testMain() async {
225225
paragraph.layout(const ParagraphConstraints(width: 800.0));
226226
expect(paragraph.plainText, 'abcdef');
227227
final List<Element> spans =
228-
paragraph.toDomElement().querySelectorAll('flt-span').cast<Element>();
228+
paragraph.toDomElement().querySelectorAll('flt-span').cast<Element>().toList();
229229
expect(spans[0].style.fontFamily, 'Ahem, $fallback, sans-serif');
230230
// The nested span here should not set it's family to default sans-serif.
231231
expect(spans[1].style.fontFamily, 'Ahem, $fallback, sans-serif');

0 commit comments

Comments
 (0)