Skip to content

even without dart:html anywhere in the code it throws conflict with native dart:html type #46248

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

Closed
jodinathan opened this issue Jun 3, 2021 · 24 comments
Labels
area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. P2 A bug or feature request we're likely to work on web-dart2js web-js-interop Issues that impact all js interop

Comments

@jodinathan
Copy link

Dart SDK version: 2.13.1 (stable) (Fri May 21 12:45:36 2021 +0200) on "macos_x64"
build_web_compilers 3.0.0

I am not using dart:html and it still complains when I try to make a JS wrapper around MediaStream: Error: JS interop class 'MediaStream' conflicts with natively supported class 'MediaStream' in 'dart:html'.

@jakemac53
Copy link
Contributor

This sounds like a more general dart sdk issue? Transfering it to that repro.

@jakemac53 jakemac53 transferred this issue from dart-lang/build Jun 3, 2021
@jakemac53 jakemac53 added the area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. label Jun 3, 2021
@sigmundch
Copy link
Member

@jodinathan - indeed, we added checks for JS-interop because commonly users find later in time type errors due to conflicts of this form. Today, the checks are done before tree-shaking, so we don't know whether the app is using dart:html or not at that point in time.

@srujzs - one possibility if we would like to reduce the noise here would be to try to detect whether there are any dart:html imports as an approximation.

@jodinathan - another thought. If you intend to always compile without support for dart:html, for example, if you are compiling with dart2js to run in nodejs, then one option here is to run dart2js with --server-mode. That will force dart2js to use an SDK that excludes dart:html and all related libraries, and will give you errors if you tried to import dart:html.

@sigmundch sigmundch added P2 A bug or feature request we're likely to work on web-dart2js web-js-interop Issues that impact all js interop labels Jun 3, 2021
@jodinathan
Copy link
Author

@sigmundch I don't think that using --server-mode is an option because I guess we lose other stuff like pub serve (webdev serve) which uses DDC but I am not sure.

This topic is very, very important to us because we are pondering in using Dart for a very big project (millions revenue) that will lead to a new cross platform framework. Dart was a strong candidate until this.

Do you think this could be fixed?

@srujzs
Copy link
Contributor

srujzs commented Jun 3, 2021

One thing I'm worried about with the approximation approach is that while the current file doesn't use dart:html, another file might, and we might come across the conflict in some other manifestation at runtime.

With DDC, this is an issue regardless of if the error is disabled, because DDC has native types live always. So, if you were to use @JS with DDC to interop with a native class, it will ignore your @JS interface and instead use the @Native one.

This really boils down to @Native types having separate type hierarchies than JS interop types, and resolving that is complicated. But to address your question @jodinathan:

Do you think this could be fixed?

Yes, and we definitely want to. However, suppressing the conflict check isn't a great fix without addressing some of the other issues that come without the check, because that means errors may pop up in different ways.

I'd like to know more about your use case though if possible. Is there a reason for not using the MediaStream defined in dart:html? If it's because of a faulty interface or missing members, you can address them with static extension methods like Siggi mentioned here: #39753

@jodinathan
Copy link
Author

One thing I'm worried about with the approximation approach is that while the current file doesn't use dart:html, another file might, and we might come across the conflict in some other manifestation at runtime.

With DDC, this is an issue regardless of if the error is disabled, because DDC has native types live always. So, if you were to use @JS with DDC to interop with a native class, it will ignore your @JS interface and instead use the @Native one.

This really boils down to @Native types having separate type hierarchies than JS interop types, and resolving that is complicated. But to address your question @jodinathan:

Do you think this could be fixed?

Yes, and we definitely want to. However, suppressing the conflict check isn't a great fix without addressing some of the other issues that come without the check, because that means errors may pop up in different ways.

I'd like to know more about your use case though if possible. Is there a reason for not using the MediaStream defined in dart:html? If it's because of a faulty interface or missing members, you can address them with static extension methods like Siggi mentioned here: #39753

Two reasons:

  • dart:html has bugs not only with MediaStream but some other things and you end up using js_util to amend that
  • Anything you do with dart:html generates a heavy boilerplate. For example, .innerHtml has sanitizing stuff. We would like to avoid that.

We are designing and about to develop a framework pretty much like Svelte and we are checking if it is possible to use Dart instead of TS or Svelte.
Then we would use conditional imports to make it work with Flutter. The idea is to aim for the web and let Flutter handle the mobile and desktop version.
However, for this to work, we need to know if is it able to use pure Dart to make web apps.
We were able to run dart in an Android 4.4 KitKat by just adding a polyfill to Map of ES6.

@sigmundch
Copy link
Member

Thanks for sharing more details. Indeed, our long term plan is to revamp dart:html to make it more lightweight, hopefully based on js-interop as much as it can be, and make it easier to maintain. So bridging the gap between js-interop and dart:html is an important goal for us.

Input like this is very helpful, so if you have additional details on ares of dart:html that are specifically too much boilerplate, let us know.

I'm sure you are also aware, but just in case, I should highlight that flutter also supports the web, in case your goal is to share more logic between web and mobile long term.


Circling back to your original issue: it would be great to figure out a way for you to be unblocked, even if the current solution is not perfect.

Some options we have discussed:
(a) reexport and patch dart:html on your application: it sounds like this is in a way what you have been doing. You can keep APIs that work, and expose new APIs to fix what doesn't work via js_util. If you use static-extension-methods this could improve the experience of using the APIs (like this example).

(b) a flag to disable this error: as @srujzs mentioned, such change will surface inconsistencies and sharp corners hard to navigate. For example, consider this program:

@JS('MediaDevices')
class MediaDevices {
  external factory MediaDevices();
  external String get id;
}

If we didn't give you the static error, you may find some surprises like:

  • (MediaStream() as dynamic) as MediaStream fails with a runtime cast error! This is because the dart:html definition is taking precedence, even if you don't import it (except in server-mode).
  • Dynamic calls will invoke methods in the underlying dart:html implementation and not the JS-interop implementation.
  • Interface calls will have inconsistent behavior: DDC will invoke the JS-interop version, but dart2js will invoke the dart:html native version.

(c) update the type system and relax the error: We need to investigate this, but possibly dart2js and DDC type systems could consider native types subtypes of some JSInterop types. It's tricky because we can't make JSinterop types include all native types, since doing so will conflate all JS-interop methods on top of native types and sometimes give conflicting and surprising behavior (e.g. unclear precedence when both native and JSinterop types define the same method). One option we are thinking of is to make this change only for JSInterop types with no instance members. Similar to the match and implement types idea proposed in #39753.

@jodinathan
Copy link
Author

@sigmundch I have, or I am trying to, roll our own html interop package here https://pub.dev/packages/js_bindings.

You mentioned a flag to disable the error. I wonder if we could have a flag to disable the use of dart:html at all?
But it is important to not lose any other web feature like dart:js_util etc.

@sigmundch
Copy link
Member

sigmundch commented Jun 15, 2021

Cool and exciting to see your approach!

You mentioned a flag to disable the error. I wonder if we could have a flag to disable the use of dart:html at all?

That is what --server-mode does in dart2js, but you are correct this is not available for DDC. It is surprisingly not that trivial to add to DDC because of it's modular nature (build systems like webdev need to understand about this and expect different states depending on whether the mode is enabled).

A middle ground could be to have DDC enforce no-imports, but continue to include dart:html in the output. DDC has fewer risks of invoking the wrong code, so it could work as an approach. Another way to enforce this outside DDC could be to define a custom lint for your projects to ensure no one imports dart:html.

The big caveat with all these approaches is that the don't compose well. If you release packages using js_bindings and other developers want to consume them, they also can't use dart:html.

We want to explore further the idea (c) above.

This approach, however, will require changing the structure of js_bindings and how it is intended to be used. The change would be to make it more static, by declaring all APIs using static extension methods. As long as js_binding is meant to be used with static types, then this approach should be viable.

To make this concrete, instead of:

@JS()
class MediaDevices withEventTarget { 
  external factory MediaDevices();
  external Promise<MediaStream> getUserMedia([MediaStreamConstraints? constraints]);
  ...
}

You'd need to have:

@JS()
class MediaDevices { 
  external factory MediaDevices();
}

extension MediaDevicesAPI on MediaDevices {
  Promise<MediaStream> getUserMedia([MediaStreamConstraints? constraints]) => callMethod(this, 'getUserMedia', [constraints]);

  // Note: eventually we will support the external syntax within static extension methods:
  //    external Promise<MediaStream> getUserMedia([MediaStreamConstraints? constraints]);
  // but for now we need to use js_util until that gets implemented.
}

At the use-site, code using MediaDevices as the static type of a local variable will work just like before.

This change would be very aligned with the direction that JS-interop is moving towards. We plan to have better language support and syntax, so it's not quite the same, but the basic idea that JS-interop methods are only available with static types is a center piece of the design. That is the mechanism we will have to allow renames as well as pre processing of arguments and post processing of results (e.g. to convert promises to futures like you did in your Promise extension).

@jodinathan
Copy link
Author

@JS()
class MediaDevices { 
  external factory MediaDevices();
}

extension MediaDevicesAPI on MediaDevices {
  Promise<MediaStream> getUserMedia([MediaStreamConstraints? constraints]) => callMethod(this, 'getUserMedia', [constraints]);

  // Note: eventually we will support the external syntax within static extension methods:
  //    external Promise<MediaStream> getUserMedia([MediaStreamConstraints? constraints]);
  // but for now we need to use js_util until that gets implemented.
}

I really like this approach but I have a question: what about boilerplate?
Considering js_bindings use this approach, would it increase the final js bundle because of the extension and use of js_util?

@sigmundch
Copy link
Member

If I understand correctly, your concern is about the final output JavaScript code size, correct?

We have active work to make this be pretty lean and efficient:

  • We have a few lowering optimizations in progress for the js_util methods, so that when we can, we will reduce them to a simpler expression that can be simplified away. For example in a few weeks we expect to land optimizations for the most common js_util methods, as long as the parameter structure is statically understandable by the compiler (e.g. the arguments are directly in a list literal, the method name is a string literal as well). This optimization will be applied in both DDC and dart2js.
  • Dart2js heavily uses inlining and can remove static method invocations easily. Static extension methods get converted to regular static methods, so many of them can be inlined away as well.

Putting these together, we expect that something like js_util.callMethod(o, 'foo', [1, 2]) will end up generating JavaScript like: o.foo(1, 2).

@jodinathan
Copy link
Author

If I understand correctly, your concern is about the final output JavaScript code size, correct?

that is exactly my concern.

e.g. the arguments are directly in a list literal, the method name is a string literal as well.
Putting these together, we expect that something like js_util.callMethod(o, 'foo', [1, 2]) will end up generating JavaScript like: o.foo(1, 2).

so if I change my js_bindings package to use js_utils like mentioned it would work like o.foo?
would this be aligned for example?

extension MediaDevicesAPI on MediaDevices {
  Promise<MediaStream> getUserMedia([MediaStreamConstraints? constraints]) => callMethod(this, 'getUserMedia', [constraints]);

thanks for the info, very interesting!

@sigmundch
Copy link
Member

/cc @rileyporter

Precisely! Just to be clear, that would be mostly in dart2js, in DDC you'll see additional calls/indirection at first.

After some js_util optimizations, we will also be adding support for external members in extensions. Once we do that, you should be able to use external again:

extension MediaDevicesAPI on MediaDevices {
  external Promise<MediaStream> getUserMedia([MediaStreamConstraints? constraints]);

Because these are statically dispatched and understood by the compilers, we finally have a way to @JS rename members, use index operators, and process inputs/results. For example:

extension MediaDevicesAPI on MediaDevices {
  // directly convert promise to futures:
  Future<MediaStream> getUserMedia([MediaStreamConstraints? constraints]) =>
    js_utils.promiseToFuture(js_utils.callMethod(this, 'getUserMedia', [constraints]));

@jodinathan
Copy link
Author

awesome!

I will change js_bindings to use extensions and will get back here once it is done

@donny-dont
Copy link

donny-dont commented Jun 17, 2021

Hey @sigmundch will these optimizations also apply to dart:js or is it only for package:js_util?

Similarly to @jodinathan I was experimenting with a dart:html replacement but I was specifically geared towards Custom Element support.

As an example here's the Element extension.

import 'dart:js';

extension ElementJsObject on JsObject {
  bool get isInstanceOf => instanceof(context['Element'] as JsFunction);

  Object get attributes => this['attributes'] as Object;

  String get tagName => this['tagName'] as String;

  String get className => this['className'] as String;
  set className(String value) {
    this['className'] = value;
  }

  String get id => this['id'] as String;
  set id(String value) {
    this['id'] = value;
  }

  String get slot => this['slot'] as String;
  set slot(String value) {
    this['slot'] = value;
  }

  Object? get shadowRoot => this['shadowRoot'];

  Object attachShadow(JsObject init) =>
      callMethod('attachShadow', <Object?>[init]) as Object;
}

Then there's a Dart implementation of Element that wraps the JsObject.

/// Browser implementation of [Element].
class ElementImpl extends NodeImpl
    with
        ParentNodeImpl,
        ChildNodeImpl,
        NonDocumentTypeChildNodeImpl,
        SlotableImpl
    implements Element {
  /// Creates an instance of [ElementImpl] from the [jsObject].
  ElementImpl.fromJsObject(JsObject jsObject) : super.fromJsObject(jsObject);

  @override
  String get tagName => jsObject.tagName;

  @override
  String get id => jsObject.id;
  @override
  set id(String value) {
    jsObject.id = value;
  }

  @override
  String get className => jsObject.className;
  @override
  set className(String value) {
    jsObject.className = value;
  }

  @override
  late Map<String, String> attributes = NamedNodeMap.fromJsObject(
      JsObject.fromBrowserObject(jsObject.attributes));

  @override
  String get slot => jsObject.slot;
  @override
  set slot(String value) {
    jsObject.slot = value;
  }

  @override
  ShadowRoot? get shadowRoot =>
      safeShadowRootFromObjectNullable(jsObject.shadowRoot);

  @override
  ShadowRoot attachShadow() =>
      ShadowRootImpl.fromJsObject(JsObject.fromBrowserObject(
        jsObject.attachShadow(
          ShadowRootInitOptionsJsObject.construct(
            mode: 'open',
            delegatesFocus: false,
          ),
        ),
      ));
}

And if you look over in https://github.com/rampage-dart/rampage/tree/main/html/web there's a Custom Element that does a click counter which runs when compiled with DDC or dart2js.

Happy to see I came to a similar conclusion with using extensions. My major concerns are how this can support Custom Elements and also how quality of life improvements like https://api.dart.dev/stable/2.13.3/dart-html/Element/on.html can be done with this approach to js-interop.

@sigmundch
Copy link
Member

Hi @donny-dont

... will these optimizations also apply to dart:js or is it only for package:js_util?

Only js_util. There is a big difference between the two: js_util doesn't wrap anything, whereas dart:js creates a wrapper for every object it accesses. Our hope with the future "extension types" is that it gives an API that feels like it wraps, but without the cost (in fact, one of the proposals for this features is called "zero-cost wrappers")

I was experimenting with a dart:html replacement but I was specifically geared towards Custom Element support.

Cool! Indeed once we have the new feature, I expect the code may look very similar to what you have in ElementJsObject. For instance, if you were to replace JsObject with a future JavaScriptObject type that is wrapper-less version of JsObject, you may be good to go.

Until we have the new feature, though, our intermediate step is to use package:js declarations instead. Would it work in your case to do:

@JS()
@anonymous
class ElementJsObject {}

extension ElementJsObjectExtension on ElementJsObject {
  ... /// everything you have in your ElementJsObject today, but replace all uses of `context` to use `js_util` instead?
}

I haven't put much thought into this, but I also wonder whether we could eliminate the second wrapper with ElementImpl. Ideally all these layers of abstraction are there to write typed code, but we would love to reduce/eliminate them if we can. If you can write ElementImpl as a class with only a constructor, and move everything else to an extension, then it may be possible to also use an extension type for it in the future.

@donny-dont
Copy link

Hey @sigmundch I hand rolled some JS interop using package:js_util in my fork of Jenny's original dart-custom-elements-demo.

https://github.com/donny-dont/dart-custom-element-demo/tree/js-util

It has the click counter working. It has Mutation Observers working. It has Events working.

Please take a look at it at your earliest convenience to see if I got things going how you all envision JS interop working in the future. I have my WebIDL parser going and am using code_builder to generate code off of that to automate the whole process. I'll modify that according to any changes you all suggest and go then should be able to generate bindings that will allow Web Components to be built in Dart.

Thanks in advance!

@sigmundch
Copy link
Member

Thanks @donny-dont - that's very cool.

I took a brief look and precisely, the pattern of JS-interop using an empty class with extensions matches what we are going after.

While browsing around, I noticed a couple things I want to point out:

  • Our current plan is that types will not be reified. This is partially true already with dart2js and DDC. Eventually, the effect is that any type you write that is JS-interop would be replaced by a simple JavaScriptObject type. This may affect a couple things in your current prototype:

  • I noticed that lib/browser declares wrappers on top of the js types. I'm curious to explore a wrapper-less approach. For example, could it work to expose the underlying JavaScript custom element instance as a JSinterop type and remove the dartWrapper?

Cheers!

@donny-dont
Copy link

Thanks for taking a look @sigmundch !

So for the is checks the fix would be to use js_util to do something like then?

extension ElementImpl on Element {
  bool get isInstanceOf => instanceof(this, context['Element']);
}

With regards to the wrapper-less version that's something I could experiment with but it seemed like a non-starter with how the JS Interop is currently. Jenny mentioned "Dart classes should be able to extend a JS interop class (e.g. for custom elements), and vice versa if possible (probably opt-in from the Dart class, so it explicitly exports itself for JS subclassing)" in her list for JS Interop improvements. Also I'm not sure how one could do wrapper-less code with quality of life improvements like making HTMLCollection into a List<Element> or how to turn it into a Dart hierarchy with it all.

Happy to work with you all to dog food more of this if you have any specific ideas to try out. Also the code is there if anyone wants to monkey with it directly.

@srujzs
Copy link
Contributor

srujzs commented Jul 28, 2021

So for the is checks the fix would be to use js_util to do something like then?

Right, with the caveat that js_util.instanceOf doesn't need to use context from dart:js.

Jenny mentioned "Dart classes should be able to extend a JS interop class (e.g. for custom elements), and vice versa if possible (probably opt-in from the Dart class, so it explicitly exports itself for JS subclassing)" in her list for JS Interop improvements.

I don't believe this will be feasible with the extension types model (and not possible with package:js today). Being able to export Dart types is still unexplored, unfortunately, and I know it's been a request for a while now. I wonder if with tear-off constructors being introduced to the language, this might affect custom elements, but not sure yet.

Also I'm not sure how one could do wrapper-less code with quality of life improvements like making HTMLCollection into a List or how to turn it into a Dart hierarchy with it all.

Yeah, I think supporting List-like capabilities will be difficult without wrappers and while extension types allow composability, they don't have the guarantees inheritance affords.

I believe that for the most part, extensions types seems still compatible with your library (and migrating off of dart:js will be doable), but I don't think it improves the situation in utilizing custom elements either (sorry!).

@jodinathan
Copy link
Author

@srujzs I've fixed js_bindings and the example is now working.
I am trying now to make a working example of webcomponents.

Tried using constructor tear-off but didn't work:

import 'package:js_bindings/js_bindings.dart';

class Foo implements HTMLElement {
  Foo() {
    // final pElem = document.createElement('p');
    // pElem.textContent = getAttribute('text');
    //
    // final shadowRoot = attachShadow(ShadowRootInit(mode: 'open'));
    // shadowRoot.appendChild(pElem);
  }
}

void main() {
  window.customElements.define('foo-cmp', allowInterop(Foo.new));
}

Error: Failed to construct 'CustomElement': The result must implement HTMLElement interface

Compiling -01 and reading the generated main.dart.js it seems that Foo is aways a function and not a class implements HTMLElement.

Could I somehow force Foo to be compiled as a class and also extend HTMLElement?
HTMLElement is a js interoped class in the package js_bindings so I can't make Foo extend it directly, only implement.

@donny-dont
Copy link

I thought we were pretty much blocked on making a wrapper-less object without dart-lang/language#1474

@srujzs
Copy link
Contributor

srujzs commented Feb 1, 2022

Custom Elements/Web Components is indeed a pain point, and that won't work as expected. There's a bit of back and forth between Siggi and Don above about possibly implementing code to handle custom elements. They, iirc, also had some current mechanisms to do this (although I can only assume it's painful today unfortunately). I haven't taken a look at the work there in a while, how would adding views/JavaScriptObject improve this beyond what was mentioned above (about replacing extensions with views and such), Don? I'm curious because we might end up taking a deeper look into this post-views to see if we can improve this use case.

@jodinathan
Copy link
Author

@donny-dont it would be feasible if the compiled JS was more strict regarding the function it creates from classes: #48269

@srujzs is there any pragma or something like that that could help generating the needed structure?

@jodinathan
Copy link
Author

yeah, it's just too complicated to use dart2js and webcomponents. I was able to make it work by doing some magical JS, even without wrappers, however, hacking like this is just not right.

Maybe I could make a builder... Even so, just too much work for something relatively simple.

Well, I am closing this issue because @staticInterop fixed it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. P2 A bug or feature request we're likely to work on web-dart2js web-js-interop Issues that impact all js interop
Projects
None yet
Development

No branches or pull requests

5 participants