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

Moving from @staticInterop type to interop extension type leads to an extra apply call #54862

Closed
srujzs opened this issue Feb 8, 2024 · 3 comments
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. web-dart2js web-js-interop Issues that impact all js interop

Comments

@srujzs
Copy link
Contributor

srujzs commented Feb 8, 2024

Compiling this:

import 'dart:js_interop';

import 'dart:html' as html;

@JS()
@staticInterop
class Node {}

extension NodeExtension on Node {
  external Node removeChild(Node child);
  external Node? get parentNode;
}

/*
@JS()
extension type Node._(JSObject _) {
  external Node removeChild(Node child);
  external Node? get parentNode;
}
*/

void main() {
  final nodes = html.window.document.nodes;
  final node = nodes[0] as Node;
  node.parentNode?.removeChild(node);
}

leads to:

    main() {
      var node,
        t1 = window.document.childNodes;
      if (0 >= t1.length)
        return A.ioore(t1, 0);
      node = t1[0];
      t1 = type$.nullable_JavaScriptObject._as(node.parentNode);
      if (t1 != null)
        type$.JavaScriptObject._as(t1.removeChild(node));
    },

Replacing the @staticInterop definition with the interop extension type results in this:

    main() {
      var node,
        t1 = window.document.childNodes;
      if (0 >= t1.length)
        return A.ioore(t1, 0);
      node = t1[0];
      t1 = type$.nullable_JSObject._as(node.parentNode);
      if (t1 != null)
        type$.JSObject._as(t1.removeChild.apply(t1, [node]));
    }

where the difference is that instead of directly calling removeChild(node), we're now doing a removeChild.apply(t1, [node]). It isn't clear why this is the case - the only real functional difference that I can tell is that one is a JavaScriptObject (the @staticInterop type), while the other is a JSObject. Erasing @staticInterop types as JSObject doesn't result in the longer call either, so there must be something else here.

cc @leonsenft

@srujzs srujzs added web-dart2js web-js-interop Issues that impact all js interop area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. labels Feb 8, 2024
@leonsenft
Copy link

Is this only because we're using nodes from dart:html? Is apply() not used if we use package:web instead?

@srujzs
Copy link
Contributor Author

srujzs commented Feb 8, 2024

That's a good question - I can't quite get the equivalent code, but this is good enough:

import 'dart:js_interop';

@JS()
external Window get window;

@JS()
@staticInterop
class Window {}

extension WindowExtension on Window {
  external Node get document;
}

@JS()
@staticInterop
class Node {}

extension NodeExtension on Node {
  external Node removeChild(Node child);
  external Node? get parentNode;
  external Node get firstChild;
}

/*
extension type Window._(JSObject _) {
  external Node get document;
}

@JS()
extension type Node._(JSObject _) {
  external Node removeChild(Node child);
  external Node? get parentNode;
  external Node get firstChild;
}
*/

void main() {
  final node = window.document.firstChild;
  node.parentNode?.removeChild(node);
}

With @staticInterop:

    main() {
      var t1 = type$.JavaScriptObject,
        node = t1._as(t1._as(t1._as(self.window).document).firstChild),
        t2 = type$.nullable_JavaScriptObject._as(node.parentNode);
      if (t2 != null)
        t1._as(t2.removeChild(node));
    },

With interop extension types:

    main() {
      var t1 = type$.JSObject,
        node = t1._as(t1._as(t1._as(self.window).document).firstChild),
        t2 = type$.nullable_JSObject._as(node.parentNode);
      if (t2 != null)
        t1._as(t2.removeChild.apply(t2, [node]));
    }

So it seems like the answer is no, this is not because of dart:html. I'll update the title to reflect that.

@srujzs srujzs changed the title Moving from @staticInterop type to interop extension type when using dart:html's window.document.nodes leads to an extra apply call Moving from @staticInterop type to interop extension type leads to an extra apply call Feb 8, 2024
@sigmundch
Copy link
Member

Interesting.

A couple thoughts. By default external methods are lowed to callMethod (from js_util, maybe we should change that to use js_interop_unsafe in the future). Then callMethod can be lowered even further to use callMethodUnchecked0 in certain cases.

Tweaking your example a bit:

import 'dart:js_interop';

@JS()
external A get a;

@JS()
@staticInterop
class A {}

extension Ax on A {
  external A m1(A? child);
}

@JS()
external B get b;

extension type B._(JSObject o) {
  external B m2(B? child);
}

extension Bx on B {
  external B m3(B? child);
}

main() {
  a.m1(a);
  b.m2(b);
  b.m3(b);
}

We generate the direct call to m1, but the apply call to m2 and m3. So I don't believe this is because of how the members are declared themselves, but more likely because of criteria we use to decide whether the extra lowering is permitted (e.g. whether the inputs to the method are safe to bypass the assertInterop check).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. web-dart2js web-js-interop Issues that impact all js interop
Projects
None yet
Development

No branches or pull requests

3 participants