dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.21k stars 1.57k forks source link

implement custom elements correctly in ddc #27311

Closed jmesserly closed 5 years ago

jmesserly commented 8 years ago

From @jmesserly on April 21, 2016 17:41

split from https://github.com/dart-lang/dev_compiler/issues/517

custom elements in our dart:html are implemented in some way that depends on dart2js interceptors. But we should be able to do something much more like how ES6 classes are used with custom elements.

_Copied from original issue: dart-lang/devcompiler#521

jmesserly commented 8 years ago

cc @dam0vm3nt who hit this. What you may be able to do is use JS interop to access the "real" custom element registration in the DOM (bypassing dart:html) and try to register the element class that way. I'm not sure our constructors will be compatible but that should be a start.

jmesserly commented 8 years ago

From @dam0vm3nt on August 11, 2016 15:10

ok, tnx. that means new JsObject.fromBrowserObject(document).call('registerElement',['my-tag',MyDartType]) ?

jmesserly commented 8 years ago

From @dam0vm3nt on August 11, 2016 15:12

I can also making a js helper that can also "unwrap" the dart type (i.e. calling dart.unwrapType(x))

jmesserly commented 8 years ago

From @donny-dont on September 12, 2016 19:27

@jmesserly @jacob314 is there anything that outside contributors can do to get this rolling?

Custom Elements V1 is enabled in Canary by default and it would be great to get things hooked in to test.

jmesserly commented 8 years ago

yeah it's probably some small tweaks to DDC's html_dart2js.dart file. Needs someone to debug what is going wrong. IIRC it was calling some dart2js code that doesn't exist in DDC. It may be as simple as ripping those calls out, but without trying I'm not sure.

jmesserly commented 8 years ago

From @donny-dont on September 12, 2016 19:34

The interceptor stuff looked like that was how it would find the created constructor as well as the attached and detached methods. I'll see if I can find anything that's being passed in that might be of interest.

jmesserly commented 8 years ago

From @donny-dont on September 12, 2016 21:30

@jmesserly @jacob314 so here's a quickie experiment.

  html$._registerCustomElement = function(context, document, tag, type, extendsTagName) {
    let unwrapped = dart.unwrapType(type);
    let proxy = new Proxy(unwrapped, {
      construct: function(target, argumentsList, newTarget) {
        return new unwrapped.created();
      }
    });
   window.customElements.define(tag, proxy);
  };

The construct method from the proxy is being called when doing new html.Element.tag('x-test'). Actually removing the proxy and using window.customElements.define(tag, unwrapped.created) also seems to work.

The problem then becomes the created constructor on Element and Node. Some of the Symbol(dartx.*) are causing problems. The first one I hit was this[dartx.offsetParent] = null. This causes an error like this Cannot set property Symbol(dartx.*) of [object Object] which has only a getter.

If you just blindly comment out the entire contents of Element and Node then you end up with The result must implement HTMLElement interface.

Is there a particular strategy for doing the interop with HTML elements? I'm just wondering if the DOM stuff in general needs to be gutted for dev compiler.

jmesserly commented 8 years ago

yeah the existing dart:html complexity definitely needs to be gutted.

Trying to set offsetParent does sound like a bug. How'd you hit that?

jmesserly commented 8 years ago

From @donny-dont on September 12, 2016 22:6

The first thing that came up with some googling was related to https://developers.google.com/web/updates/2015/04/DOM-attributes-now-on-the-prototype-chain.

  dart.defineNamedConstructor = function(clazz, name) {
    let proto = clazz.prototype;
    let initMethod = proto[name];
    let ctor = function(...args) {
      initMethod.apply(this, args);
    };
    ctor[dart.isNamedConstructor] = true;
    ctor.prototype = proto;
    dart.defineProperty(clazz, name, {value: ctor, configurable: true});
  };

From what's happening here it looks like that might be the issue.

Other thing I noticed was the DivElement constructor was never getting called in DDC compiled code when doing new html.Element.div(). That's what got me wondering what sort of state the HTML interop was with DDC. Actually if you do new html.DivElement.created(), not that I thought you should, it'll error.

jmesserly commented 8 years ago

well constructor shouldn't be called, it should call createElement ultimately right?

jmesserly commented 8 years ago

but that's a fair point to @jacob314 -- we should skip generating a constructor for native types. This kind of thing is why this bug is still open :)

jmesserly commented 8 years ago

From @donny-dont on September 12, 2016 22:22

First time really digging into this so I had assumed that there would be some sort of wrapping going on for things like streams and other dart:html niceties. Hadn't found where that was happening yet.

jmesserly commented 8 years ago

IIRC they're all overlays not wrappers. We use ES6 symbols so we can add props in a safe way to HTML types.

jmesserly commented 8 years ago

That should ultimately make Custom Elements easier, once we're over the growing pains :)

jmesserly commented 8 years ago

From @donny-dont on September 12, 2016 22:38

Ah okay so the adding of props happens like this then?

  dart.setSignature(html$.Node, {
    constructors: () => ({
      _created: dart.definiteFunctionType(html$.Node, []),
      _: dart.definiteFunctionType(html$.Node, [])
    }),
    fields: () => ({
      [dartx.childNodes]: ListOfNode(),
      [dartx.baseUri]: core.String,
      [dartx.firstChild]: html$.Node,
      [dartx.lastChild]: html$.Node,
      [_localName]: core.String,
      [_namespaceUri]: core.String,
      [dartx.nextNode]: html$.Node,
      [dartx.nodeName]: core.String,
      [dartx.nodeType]: core.int,
      [dartx.nodeValue]: core.String,
      [dartx.ownerDocument]: html$.Document,
      [dartx.parent]: html$.Element,
      [dartx.parentNode]: html$.Node,
      [dartx.previousNode]: html$.Node,
      [dartx.text]: core.String
    }),
jmesserly commented 8 years ago

that's the runtime type info (for tear-offs, dynamic call checks). I believe it's something like "defineExtensionMembers" or something of that nature. It takes the real DOM type and the fake type we've created and merges the properties in

jmesserly commented 8 years ago

From @vsmenon on September 12, 2016 22:45

That's more a record of what happened for mirrors or dynamic ops. See the registerExtension function. That's where the Dart members are injected on the underlying JS ones for overlayed types.

jmesserly commented 8 years ago

From @donny-dont on September 12, 2016 23:34

Thanks @vsmenon and @jmesserly. Its definitely making more sense how its setup now.

So if you comment out the contents of created in Element and Node. You can do the following and get something back.

    static createElement_tag(tag, typeExtension) {
      if (typeExtension != null) {
        return document.createElement(tag, typeExtension);
      }
      var constructor = window.customElements.get(tag);

      if (constructor) {
        return new constructor();
      } else {
        return document.createElement(tag);
      }

The thing you end up getting back from document.createElement isn't quite right though. The prototype chains are mentioning HtmlElement not HTMLElement so something is going wrong.

jmesserly commented 8 years ago

From @donny-dont on September 13, 2016 20:17

So should HTMLElement and the like be mixed in with the html$.HtmlElement? Not sure if that would do the trick for this.

jmesserly commented 8 years ago

yeah html$.HtmlElement should be mixed into HTMLElement.

For your custom element, you want it to inherit from HTMLElement most likely. Likely need a tweak in the compiler to understand that when user classes inherit from "native" SDK classes, they need to use the "native" type not the Dart type.

jmesserly commented 8 years ago

From @vsmenon on September 13, 2016 21:1

Alternatively, in Dart, define:

class MyFooInDart extends HtmlElement { ... }

in JS, something like:

class MyFooInJs extends HTMLElement { ... }
dart.registerExtension(MyFooInJs, MyFooInDart);
customElements.define("my-foo", MyFooInJs);
dam0vm3nt commented 8 years ago

@vsmenon : I've tryied that but it doesn't work because MyFooInDart methods and properties wont be available in MyFooJs, while

...
customElement.define('my-foo',MyFooInDart);

ends up in the usual dart_sdk.js:40014 Uncaught TypeError: Cannot set property Symbol(dartx.offsetParent) of [object Object] which has only a getter problem.

jmesserly commented 6 years ago

We'll need to revisit this for DDC and dart2js after 2.0 ... it's more an issue of how the HTML library should work and what kinds of JS interop to support, rather than DDC specific (though there's certainly compiler-specific issues, due to the different JS object layout. We need to reconcile that between DDC and dart2js.)

jifalops commented 6 years ago

Is it currently possible to create/register custom elements in Dart 2.0? (Edit: With DDC)

jmesserly commented 6 years ago

I think JS interop is the safest way right now. I made an example with a JS custom element and a Dart class that wraps it: https://github.com/jmesserly/dart-custom-element-demo/

It doesn't seem to work in dart2js though, at least with checks turned on (I think dart2js doesn't understand how to do JS interop against a custom element).

jifalops commented 6 years ago

Thanks for this demo, I'm going to see if I can get it to work with dart2js. Is it required for the JS component to provide a createInstance() method?

This js_interop testing package indicates web component can be consumed, but not created.

jmesserly commented 6 years ago

createInstance() is how I linked up the Dart class with the JS custom element, so they can reference each other. There are probably other ways to structure that part.

donny-dont commented 6 years ago

Would that scale for a large number of elements though? Seems like dart:html has to have a better idea on what custom elements are now.

jmesserly commented 6 years ago

Would that scale for a large number of elements though? Seems like dart:html has to have a better idea on what custom elements are now.

You could reuse the same JS interop class for all of your Dart custom elements, if desired.

A few things happened here. Biggest one is that the custom element spec changed a bit from the early versions, and it's harder to make it work natively in Dart now (mostly because of constructors). "dart:html" is complex because it renames/reimplements a bunch of the DOM, and requires a lot magic in the compilers to support it. That makes it hard for normal Dart classes to subclass DOM classes.

I'm happy with where the custom element spec ended up--it's a lot cleaner than earlier versions, IMO. In the long run, it probably helps us :). In the short term, we need to improve JS interop & DOM support for dart2js/dartdevc before it'll work nicely.

jifalops commented 6 years ago

Right now, registering/defining custom elements in Dart works, but only in dart2js. Consuming JS web components only works with DDC, unless they are first compiled and globally accessible. (Just documenting this.)

donny-dont commented 6 years ago

Would that scale for a large number of elements though? Seems like dart:html has to have a better idea on what custom elements are now.

You could reuse the same JS interop class for all of your Dart custom elements, if desired.

Wouldn't that have problems with different tags though?

A few things happened here. Biggest one is that the custom element spec changed a bit from the early versions, and it's harder to make it work natively in Dart now (mostly because of constructors). "dart:html" is complex because it renames/reimplements a bunch of the DOM, and requires a lot magic in the compilers to support it. That makes it hard for normal Dart classes to subclass DOM classes.

Right. The constructor stuff was definitely built around how v0 was setup. On top of that there seemed to be a lot of internal JS bits that made it all actually work.

I'm happy with where the custom element spec ended up--it's a lot cleaner than earlier versions, IMO. In the long run, it probably helps us :). In the short term, we need to improve JS interop & DOM support for dart2js/dartdevc before it'll work nicely.

Yea the spec is a lot nicer now. I really like how slot panned out.

I know there was talk about leaving a dart:html like library up to the community. Would be nice to have a way to roll your own bindings considering how much the web changes. The extension method proposal looks nice for potentially doing some of that.

Anyways great to see some movement here. I really don't want to have to program in TypeScript 😉

jmesserly commented 6 years ago

Wouldn't that have problems with different tags though?

You'd need one per unique base class, yeah.

[...] Would be nice to have a way to roll your own bindings considering how much the web changes. The extension method proposal looks nice for potentially doing some of that.

Yup. If we can make it work more like a normal package (based on JS interop), then anyone can make bindings. The browser & specification environment has changed significantly since the Dart DOM bindings were created, and it's much better now. I think we can find a simpler way of talking to the DOM.

donny-dont commented 6 years ago

Yup. If we can make it work more like a normal package (based on JS interop), then anyone can make bindings. The browser & specification environment has changed significantly since the Dart DOM bindings were created, and it's much better now. I think we can find a simpler way of talking to the DOM.

I tried to roll my own html lib like @dam0vm3nt was doing. One thing I wasn't sure of was how one could make stuff like children which made things feel more darty. Other thing was how to make it so you forwarded something like attachedCallback to attached.

elmcrest commented 5 years ago

any Ideas how to get dart2js support this, too? I've created an issue in the example repo ... https://github.com/jmesserly/dart-custom-element-demo/issues/3

elmcrest commented 5 years ago

hey again, so this will break my app, so worst case seems to happen soon...

telegram-cloud-file-2-255434779-298849-3260950519069668829

jmesserly commented 5 years ago

Merging this into #27445 (the issue about adding support for custom elements V1 in dart:html).

donny-dont commented 5 years ago

@jmesserly just a note that the issue you're merging into has a locked conversation.

eukreign commented 5 years ago

Yeah, apparently I said something that offended @matanlurey and he censored me and then locked the thread. I don't even know what I said that upset him so much ¯_(ツ)_/¯

Anyways, I'm thrilled at all of the work @jmesserly is doing to resolve this issue. Feels like there is hope for dart on the web again! Really exciting! Thanks @jmesserly !

jmesserly commented 5 years ago

just a note that the issue you're merging into has a locked conversation.

I saw that. It is still the main tracking issue, though, as far as I know. We can reopen it once there's an update. I understand there's a lot of passion about this feature, and I'll bring that up to the team so it's given due consideration.

Anprotaku commented 1 year ago

Any updates about customElement support in dart? It has been a few years now and i wonder whats going on?