Skip to content

Web Components: Shadow Dom + Template Element #295

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

Merged
merged 21 commits into from
Nov 1, 2018
Merged

Web Components: Shadow Dom + Template Element #295

merged 21 commits into from
Nov 1, 2018

Conversation

futursolo
Copy link
Contributor

@futursolo futursolo commented Oct 19, 2018

Rationale

With the landing of Web Components in Firefox 63, all major browsers will have web components support on by default in a couple of weeks(when Firefox 63 becomes stable). Web Components can provide better performance especially on mobile platforms comparing to other JavaScript-based solutions. I think now it is a good time to implement Web Components support.

Outcome

Web Components consists of Template Element, Shadow DOM, Custom Element and HTML Imports. This pull request aims at implementing all of them except HTML Imports. There are concerns about the disconnection between ES6 Modules and HTML Imports and thus Safari and Firefox refuse to implement it. Frameworks like Polymer used to use HTML Imports has moved away from it. Chrome also deprecated HTML Imports. It may need major revamp before it gains any adoption from browser vendors. Hence, I've decided to not include HTML imports in this pull request.

Steps

  • Template Element
    • TemplateElement
    • Document.import_node
    • Tests
  • Shadow DOM
    • Slot
      • SlotElement
      • ISlotable
      • SlotChangeEvent
    • ShadowRootMode
    • ShadowRoot
    • IElement.attach_shadow
    • IElement.shadow_root
    • Tests
  • Custom Elements(Withdrawn due to unexpected circumstances)

(I didn't write tests because I don't have Chrome on my computer. I'll make them up eventually.)

Unsolved Question

How should custom elements be constructed?
Approach 1:

#[derive(Clone, Debug, PartialEq, Eq, ReferenceType)]
#[reference(instance_of = "MyElement")]
#[reference(subclass_of(EventTarget, Node, Element, HtmlElement))]
#[custom_element(name = "my-element", extends = "HTMLElement")]
struct MyElement(Reference);

impl ICustomElement for MyElement { // HtmlElement and IHtmlElement are already occupied.
    fn inited(&self) {
        // JavaScript Constructor Called
    }
    fn connected(&self) {
        // HTMLElement.connectedCallback
    }
    fn attr_changed(&self, attr_name: String, before: String, after: String) {
        // HTMLElement.attributeChangedCallback
    }
    fn adopted(&self) {
        // HTMLElement.adoptedCallback
    }
    fn disconnected(&self) {
        // HTMLElement.disconnectedCallback
    }

    fn observed_attrs(&self) -> Vec<String> {
        // vec!["name".to_owned(), "class".to_owned()]
    }
}

fn define_custom_elements() {
    // ...
    window().custom_elements().define(MyElement);
    // ...
}

Pros:

  • It allows custom elements to subclass other elements(e.g.: HTMLTextareaElement).
  • It looks like the original api.

Cons:

  • I don't know how to implement this.
    • Is there any other way to construct a JavaScript class than js! macro?
    • How to downcast JavaScript types to rust structs at runtime?
    • Will [reference(instance_of = "MyElement")] work if the class is not defined at the time wasm file is initiated?

Approach 2:

fn define_custom_elements() {
    // ...
    CustomElementBuilder::new("my-element")
        .on_inited(|element| {})
        .on_connected(|| {})
        .on_attr_changed(|attr_name, before, after| {})
        .on_adopted(|| {})
        .on_disconnected(|| panic!())
        .watch_attrs(["name", "class"])
        .define().unwrap();
    // ...
}

Pros:

  • It is easy to implement. Just js! all javascript code inside define().

Cons:

  • It's not possible to subclass other elements like HTMLTextareaElement.

See also: #32

@Pauan
Copy link
Contributor

Pauan commented Oct 20, 2018

Approach 1

Is there any other way to construct a JavaScript class than js! macro?

I don't think so, at least not if you want to define custom methods.

However, I think the custom_element attribute should be able to use the js! macro, so it should be possible to generate the class in there.

How to downcast JavaScript types to rust structs at runtime?

You just have to implement TryFrom<Value> for the struct:

impl TryFrom<Value> for MyElement {
    type Error = ConversionError;

    fn try_from(value: Value) -> Result<Self, Self::Error> {
        ...
    }
}

This can easily be automated by the custom_element attribute.

Will [reference(instance_of = "MyElement")] work if the class is not defined at the time wasm file is initiated?

I'm not sure, but I don't think it matters. Since the class is being generated by stdweb, I don't think we need an instance_of at all.

Approach 2

It's not possible to subclass other elements like HTMLTextareaElement.

Is this true? I would imagine something like this would work:

CustomElementBuilder::new("my-element")
    .extends(TextAreaElement)
    .define()

And then in the define method...

if let Some(ref extends) = self.extends {
    js!(
        return class extends @{extends} {
            ...
        };
    ).try_into().unwrap()

} else {
    js!(
        return class {
            ...
        };
    ).try_into().unwrap()
}

It's not pretty, but I think it would work.

@Pauan
Copy link
Contributor

Pauan commented Oct 20, 2018

Oh, I see the issue with approach 2: stdweb doesn't currently expose the actual JS classes (only the instances of the classes). So you'd need to do something like this:

CustomElementBuilder::new("my-element")
    .extends(js!( return HTMLTextAreaElement; ))
    .define()

A bit ugly, but doable. And maybe we could figure out a cleaner solution, something like this:

CustomElementBuilder::new("my-element")
    .extends(TextAreaElement::class)
    .define()

@Pauan
Copy link
Contributor

Pauan commented Oct 20, 2018

By the way, there is a third approach: using macros.

custom_element! {
    #[derive(Clone, Debug, PartialEq, Eq, ReferenceType)]
    #[reference(subclass_of(EventTarget, Node, Element, HtmlElement, TextAreaElement))]
    pub struct MyElement extends HTMLTextAreaElement;

    fn inited(&self) {
        // JavaScript Constructor Called
    }

    fn connected(&self) {
        // HTMLElement.connectedCallback
    }

    fn attr_changed(&self, attr_name: String, before: String, after: String) {
        // HTMLElement.attributeChangedCallback
    }

    fn adopted(&self) {
        // HTMLElement.adoptedCallback
    }

    fn disconnected(&self) {
        // HTMLElement.disconnectedCallback
    }

    fn observed_attrs(&self) -> Vec<String> {
        // vec!["name".to_owned(), "class".to_owned()]
    }
}

Since it needs to use the js! macro anyways to generate the JS class, perhaps a macro is most appropriate.

@futursolo
Copy link
Contributor Author

Hmm... I still think approach 1 is the way to go. I proposed the second approach just because it's much easier to implement.

Though there are ways to workaround the subclass issue for approach 2, it is still kind of difficult to use. You still have to downcast the reference if you want to use any methods from TextareaElement. Probably #293 is a solution to this?

As for approach 3, what's the advantage of using custom_element! instead of
#[custom_element(...)]? And since attributes are technically macros, I think approach 1 & 3 should yield the same result. Also, If we are going to add a macro with extends syntax, why not just give users the ability to define JavaScript classes with Rust code in general just like what rust-cpython did?

When I originally thought about the API, I specifically avoided using macros with custom syntax for a couple of reasons:

  • It's harder for people to understand.
  • Compiler error messages are not that helpful when dealing with code inside a macro.
  • Your favourite editor will not provide proper syntax highlight to your macro.
  • Rustfmt does not know what to do with the code inside the macro.
  • It's more difficult to implement a macro with custom syntax than an attribute.
  • Rust's macro system is about to change. I think it's better to keep things simple as there will be less work when we adopt macro 2.0.

Copy link
Owner

@koute koute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

Initial review done. I'll probably still have more comments, but it's already late and I need to go to sleep. (:

Copy link
Owner

@koute koute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Continuing the review....

/// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/API/Element/attachShadow)
// https://dom.spec.whatwg.org/#ref-for-dom-element-attachshadow
fn attach_shadow( &self, mode: ShadowRootMode ) -> Result<ShadowRoot, AttachShadowError> {
js_try!(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Broken indentation (:

/// present them together.
///
/// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/slot)
// https://html.spec.whatwg.org/multipage/scripting.html#the-slot-element
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A link to the IDL would be better:

https://html.spec.whatwg.org/multipage/scripting.html#htmlslotelement


/// Setter of name.
#[inline]
pub fn set_name<S: AsRef<str>>( &self, new_name: S ) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any particular reason why you made it S: AsRef<str>? In general we usually take &str and have the user convert it themselves.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No particular reason, I just feel it would be easier to use.

///
/// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/API/HTMLSlotElement/assignedNodes)
// https://html.spec.whatwg.org/multipage/scripting.html#the-slot-element:dom-slot-assignednodes
pub fn assigned_nodes( &self, flatten: bool ) -> Vec<Node> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than take a bool parameter I'd probably split this into two methods, say, assigned_nodes and flattened_assigned_nodes? (The second name is a little long though; if you can think of a better one feel free to suggest it.) Ditto for assigned_elements.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a enum (like webapi::node::CloneKind)?

enum ChildrenArrangement { // I cannot come up with a better name.
    Flattened,
    Nested,
}

pub fn assigned_nodes( &self, a: ChildrenArrangement ) -> ... {

Also, should the return type be NodeList?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, should the return type be NodeList?

In the specs it says that it's a sequence<Node>, which is different than NodeList, so I don't think so.

How about a enum (like webapi::node::CloneKind)?

Well, that would work too, but that's another extra type which you have to import and remember.

I don't have a strong opinion either way though.

/// The mode property of the `ShadowRoot` specifies its mode.
///
/// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/API/ShadowRoot/mode)
// https://dom.spec.whatwg.org/#dom-shadowroot-mode
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to IDL: https://dom.spec.whatwg.org/#ref-for-dom-shadowroot-mode

/// the ShadowRoot is attached to.
///
/// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/API/ShadowRoot/host)
// https://dom.spec.whatwg.org/#dom-shadowroot-host
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://dom.spec.whatwg.org/#ref-for-dom-shadowroot-host

use webapi::html_elements::SlotElement;

/// The Slotable mixin defines features that allow nodes to become the contents of
/// a <slot> element — the following features are included in both Element and Text.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Escape all of the HTML tags in the doc comments, or they'll screw up the docs.

<slot> -> `<slot>`

@koute
Copy link
Owner

koute commented Oct 24, 2018

Regarding the custom elements - personally I'd prefer if they were defined using normal Rust syntax with a derive macro and/or a procedural macro generating the glue. (So option 1 I guess.)

Will [reference(instance_of = "MyElement")] work if the class is not defined at the time wasm file is initiated?

As long as the MyElement is defined in the global namespace - yes, otherwise no.

Is there any other way to construct a JavaScript class than js! macro?

Currently there are two ways to emit JS code:

  1. The js! macro.
  2. Adding a prepend-js in Web.toml.

Technically it would also be possible to introduce a third mechanism (which wouldn't necessarily have to be public) which would allow us to emit JS code in a more declarative way in the outermost namespace "before" main runs, which might be useful in this case?

@futursolo
Copy link
Contributor Author

Option 1 also have another problem. The user have no control on when the element is defined.

They may want to have it defined after load event, load some resources before defining the element or do some checks to decide whether define it or not.

We may want to add something like #[custom_element(..., deferred = true)].

@futursolo
Copy link
Contributor Author

Ah, never mind. It seems like js! won't work outside functions.

So we need a define() method that the user need to call somewhere inside a function if js! is used to implement the custom element class.

Or we need to use the "third mechanism" that @koute mentioned.

@futursolo
Copy link
Contributor Author

It seems like travis became one commit behind after I pushed a commit during the time when GitHub was unstable earlier this week. Can someone with write access trigger a manual rebuild to see if this can be fixed?

pub fn assigned_nodes( &self, kind: SlotContentKind ) -> Vec<Node> {
let is_flatten = match kind {
SlotContentKind::Default => false,
SlotContentKind::Fallback => true,
Copy link
Owner

@koute koute Oct 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the member in the specs is called flatten can you explain why are the enums named Default and Fallback instead of, I don't know, NonFlattened and Flattened? (I don't mind the discrepancy as long as it's well motivated and done for good reasons; I'm not super familiar with these APIs, so that's why I'm asking.)

Also, a related question - do you actually know which variant is used more often in practice? (Since, e.g., if one variant is used 90% of the time and the other is used only 10% of the time then it makes more sense to have two methods to have better ergonomics for the most common case, however if it's more like 50% each then an enum like this might not be a bad idea.)

Copy link
Contributor Author

@futursolo futursolo Oct 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, according to MDN, what { flatten: true } actually means is that it will return the fallback content of the slot defined in the <slot></slot> tag if nothing's been assigned using slot= attribute. And the default behaviour for slot.assignedNode() is { flatten: false } which means the browser will only return content assigned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also,

slot.assigned_node(SlotContentKind::Assigned);

looks kind of weird.

So I changed it to

slot.assigned_node(SlotContentKind::Default);

.

@koute
Copy link
Owner

koute commented Oct 25, 2018

I've restarted the stable tests on Travis and they still fail. (It looks like you might be missing an use import.)

FYI, you can always retrigger them yourself with some git trickery. (: Just run git commit --amend, add a dot at the end, run git commit --amend again, remove the dot, and run git push --force.

@futursolo
Copy link
Contributor Author

Forgive me for that messy commit history...

Well, it seems like that RUST_BACKTRACE does not work on cargo-web. So I have no choice but to spin up a vm with chromium and node to fix the issue by commenting out the code line by line.

And it turned out to be an issue with \n appearing at the beginning of the raw html tag string in which it creates a TextNode. Ah silly me...

So all tests run without any problem in my vm now, but travis is still complaining about TypeError: $1.assignedElements is not a function. Chrome Status says it's been enabled by default since 65. So now I have no idea on what's wrong with it.

It also seems like that only Chrome supports assignedElements as of now. Probably I will end up removing assigned_elements from test_shadow_dom.

@Pauan
Copy link
Contributor

Pauan commented Oct 28, 2018

As for approach 3, what's the advantage of using custom_element! instead of #[custom_element(...)]?

The biggest benefit I see is that it allows you to not specify every method. That's actually quite important: most of the time users won't need or want to implement every method.

There might be performance differences as well: if the browser notices that the class doesn't have a method, it might use a fast path which completely avoids that method call.

But since approach 1 requires the user to specify all methods, the browser now has to always invoke those methods, even if the methods don't really do anything.

And since attributes are technically macros, I think approach 1 & 3 should yield the same result.

Assuming that the user always specifies all the methods, yes the end result will be the same. But that's not the case if the user wishes to not specify some methods.

Also, If we are going to add a macro with extends syntax, why not just give users the ability to define JavaScript classes with Rust code in general just like what rust-cpython did?

I think that's a great idea! Being able to create JS classes might be useful in situations other than custom elements. I guess it would look something like this:

js_class! {
    #[derive(Clone, Debug, PartialEq, Eq, ReferenceType)]
    #[reference(subclass_of(EventTarget, Node, Element, HtmlElement, TextAreaElement))]
    pub struct Foo(TextAreaElement) extends HTMLTextAreaElement {
        pub fn inited(&self) {
            ...
        }
    }
}

It would generate this Rust code:

#[derive(Clone, Debug, PartialEq, Eq, ReferenceType)]
#[reference(subclass_of(EventTarget, Node, Element, HtmlElement, TextAreaElement))]
struct Foo(TextAreaElement);

impl Foo {
    pub fn inited(&self) {
        ...
    }
}

lazy_static! {
    pub static ref Foo: Foo = js!(
        return class extends HTMLTextAreaElement {
            inited() {
                @{Foo::inited}(this);
            }
        };
    ).try_into().unwrap();
}

impl Foo {
    pub fn new() -> Self {
        js!( return new @{&*Foo}(); ).try_into().unwrap()
    }
}

Now you can define a custom element like this:

custom_elements().define_extends("foo", &*Foo, "textarea")

Compiler error messages are not that helpful when dealing with code inside a macro.

I wonder how true that is. Especially if it's written as a proc macro, I think we can provide good error messages.

Your favourite editor will not provide proper syntax highlight to your macro.

Sublime Text works fine, as long as you're using Rust syntax inside of the macro.

Macros are quite common in Rust (e.g. lazy_static), so text editors must handle them well. I'd be surprised if there was a major text editor that didn't.

Even the Markdown syntax highlighting here on GitHub works with macros (see the Foo code above).

It's more difficult to implement a macro with custom syntax than an attribute.

I'm not sure that's true, since they both have to do similar things (and since attributes internally call into proc macros anyways).

Rust's macro system is about to change. I think it's better to keep things simple as there will be less work when we adopt macro 2.0.

It would probably be built using proc macros, not macro_rules!

@futursolo
Copy link
Contributor Author

The biggest benefit I see is that it allows you to not specify every method. That's actually quite important: most of the time users won't need or want to implement every method.

There might be performance differences as well: if the browser notices that the class doesn't have a method, it might use a fast path which completely avoids that method call.

But since approach 1 requires the user to specify all methods, the browser now has to always invoke those methods, even if the methods don't really do anything.

I think most users will not use custom elements on its own directly. They will use a framework that glues custom elements, shadow dom, and template together with optional data binding. And the framework, of course, is going to define all methods.

js_class! {
    // Why `#[derive(...)]` cannot be automated by `js_class!`?
    // Why is `pub` needed? Aren't we going to export it to JavaScript-side regardlessly?
    #[derive(Clone, Debug, PartialEq, Eq, ReferenceType)]
    #[reference(subclass_of(EventTarget, Node, Element, HtmlElement, TextAreaElement))]
    pub struct Foo(TextAreaElement) extends HTMLTextAreaElement {
         // Why this cannot be read from #[reference(instance_of = "HTMLTextAreaElement")]
        // What if there's a method called `inited` in `HTMLTextAreaElement `?
        // How do we call `super()`?
        pub fn inited(&self) {
            ...
        }
        // Do we need to use camel case for methods?
        // `rustc` would complain about that. I would complain either.
        pub fn attributeChangedCallback(&self, name: &str, before: &str, after: &str) {
            // ...
        }
    }
}

Though I said "use macro to define a JavaScript class". I am not a big fan of the way rust-cpython doing it. I think PyO3's way is better. But I don't have the need to define JavaScript classes using Rust, so I am not very interested in this topic.

Compiler error messages are not that helpful when dealing with code inside a macro.

I wonder how true that is. Especially if it's written as a proc macro, I think we can provide good error messages.

I am not talking about the syntax error on custom syntax. I am talking about the error on code inside the macro. For example, when I was implementing set_name for SlotElement, I originally did:

/// Setter of name.
#[inline]
pub fn set_name<S: AsRef<str>>( &self, new_name: S ) {
    js! ( @(no_return)
        @{self}.name = @{new_name};
    );
}

And rustc told me:

error[E0277]: the trait bound `webcore::newtype::Newtype<_, S>: webcore::serialization::JsSerializeOwned` is not satisfied
   --> src/webcore/macros.rs:306:21
    |
306 |           let $name = $crate::private::JsSerializeOwned::into_js_owned( &mut $name );
    |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `webcore::serialization::JsSerializeOwned` is not implemented for `webcore::newtype::Newtype<_, S>`
    | 
   ::: src/webapi/html_elements/slot.rs:60:9
    |
60  | /         js! ( @(no_return)
61  | |             @{self}.name = @{new_name};
62  | |         );
    | |__________- in this macro invocation
    |
    = help: the following implementations were found:
              <webcore::newtype::Newtype<(webcore::serialization::NonFunctionTag, ()), T> as webcore::serialization::JsSerializeOwned>
              <webcore::newtype::Newtype<(webcore::serialization::FunctionTag, (A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12)), F> as webcore::serialization::JsSerializeOwned>
              <webcore::newtype::Newtype<(webcore::serialization::FunctionTag, (A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12)), std::option::Option<F>> as webcore::serialization::JsSerializeOwned>
              <webcore::newtype::Newtype<(webcore::serialization::FunctionTag, (A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12)), webcore::mutfn::Mut<F>> as webcore::serialization::JsSerializeOwned>
            and 81 others
note: required by `webcore::serialization::JsSerializeOwned::into_js_owned`
   --> src/webcore/serialization.rs:59:5
    |
59  |     fn into_js_owned< 'a >( value: &'a mut Option< Self > ) -> SerializedValue< 'a >;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

I do not think this error message is useful compare to this one.

error[E0277]: the trait bound `webcore::value::Value: std::convert::From<S>` is not satisfied
  --> src/webapi/html_elements/slot.rs:60:33
   |
60 |         let n: Value = new_name.into();
   |                                 ^^^^ the trait `std::convert::From<S>` is not implemented for `webcore::value::Value`
   |
   = help: consider adding a `where webcore::value::Value: std::convert::From<S>` bound
   = note: required because of the requirements on the impl of `std::convert::Into<webcore::value::Value>` for `S`

Luckily this macro is short and I can find my error pretty quickly (I forgot to as_ref(), silly me...). Both of them didn't tell me to as_ref(), but at least the second one told me which line is wrong. If this macro is 500 lines long(Say, I implement a JavaScript class using macro), I might end up with having to rewrite the whole thing because I do not know what's wrong with it.

Your favourite editor will not provide proper syntax highlight to your macro.
Sublime Text works fine, as long as you're using Rust syntax inside of the macro.

Macros are quite common in Rust (e.g. lazy_static), so text editors must handle them well. I'd be surprised if there was a major text editor that didn't.

Even the Markdown syntax highlighting here on GitHub works with macros (see the Foo code above).

I wonder why my editor won't automatically highlight keyword extends? Probably my editor called Vim is now obsolete and won't learn how to highlight macro custom syntax on its own.

Markdown didn't highlight extends either.

It's more difficult to implement a macro with custom syntax than an attribute.

I'm not sure that's true, since they both have to do similar things (and since attributes internally call into proc macros anyways).

Do you think that it would be easier to implement

js_class! {
    #[reference(subclass_of(EventTarget, Node, Element, HtmlElement))]
    class A extends HtmlElement {  // Why `class` and `extends` are not highlighted?
    // Can we do `class extends HTMLElement` and ask `js_class!` to return the anonymous class as `Value`?
        constructor(&this) {
            js_super_constructor!(this);
            // Do we still need `js!` to access js methods of `this`?
            // ...
        }
        // ...
    }
}

than

#[custom_element("my-element")] // Extends HtmlElement by default.
#[custom_element(name = "my-element", extends = TextareaElement)]
// ...

fn main() {
    // ...
    MyElement::define().unwrap();
    // ...
}

using procedural macro?

Anyways, I think there're some common work to do regardless which method is chosen. I guess I will start with modifying #[reference(instance_of = "HTMLElement")] to preserve the class name (or implement a method to_js_class(), I still haven't decided which one yet.)somewhere so we can extends Rust structs not strings.

@Pauan
Copy link
Contributor

Pauan commented Oct 28, 2018

I think most users will not use custom elements on its own directly. They will use a framework that glues custom elements, shadow dom, and template together with optional data binding. And the framework, of course, is going to define all methods.

Could you explain more? I don't see why a framework would want to always define all the methods (especially if it has some performance impact).

In any case, we are not creating that framework, we are creating DOM bindings. And the DOM allows you to omit methods (and this omition is important), so we need to provide the ability to omit methods.

I am not a big fan of the way rust-cpython doing it. I think PyO3's way is better.

I don't mind either way, both options are good.

But I don't have the need to define JavaScript classes using Rust, so I am not very interested in this topic.

Okay, since you're not interested, we can leave that for a different PR.

I do not think this error message is useful compare to this one.

That has nothing to do with macros. The error you're getting is correct and precise, it is not obfuscated by macros.

The error is hard to understand because it's leaking internal details about the traits used by the js! macro. The same thing would happen even without a macro.

As proof that error messages work with macros, consider the lazy_static macro:

lazy_static! {
    static ref Foo: u32 = "foo";
}

I get this error:

error[E0308]: mismatched types
 --> src/main.rs:2:27
  |
2 |     static ref Foo: u32 = "foo";
  |                     ---   ^^^^^ expected u32, found reference
  |                     |
  |                     expected `u32` because of return type
  |
  = note: expected type `u32`
             found type `&'static str`

The error is exactly as it should be.

I admit the errors with js! are usually quite bad, but that's more a fault of the js! macro and not with macros in general.

I wonder why my editor won't automatically highlight keyword extends? Probably my editor called Vim is now obsolete and won't learn how to highlight macro custom syntax on its own.

Markdown didn't highlight extends either.

All of the Rust syntax is highlighted correctly, and the Rust syntax is 99% of the js_class! macro.

The custom_element attribute isn't syntax-highlighted either, it's the same situation.

Do you think that it would be easier to implement

I never said a macro would be easier. The attribute is still a proc macro, it still has to do a lot of the same work. The only thing that might be harder with js_class! is parsing the code, but I believe syn should be able to deal with that.

Also, I'm not sure why you put in a superfluous constructor, and why you omitted the reference for the attribute version. Or why you changed struct to class?

A fair comparison (with all superfluous details stripped) would look like this:

js_class! {
    struct Foo(InputElement) extends HTMLInputElement {
        // ...
    }
}

custom_elements().define_extends("foo", &*Foo, "input");
#[custom_element(name = "foo", extends = "input", inherits = "HTMLInputElement")]
struct Foo(InputElement);

impl ICustomElement for Foo {
    // ...
}

Except that the first version can omit methods, so in practice it will almost always be much shorter.

Can we do class extends HTMLElement and ask js_class! to return the anonymous class as Value?

That sounds reasonable to me. Though I'm not sure how useful it would be.

Do we still need js! to access js methods of this?

If accessing a method which is defined in the class body, it shouldn't require use of the js! macro. You can just call the methods directly.

Extends HtmlElement by default.

Custom elements do not inherit from HTMLElement by default (and they should not inherit from HTMLElement by default).

@futursolo
Copy link
Contributor Author

Could you explain more? I don't see why a framework would want to always define all the methods (especially if it has some performance impact).

In any case, we are not creating that framework, we are creating DOM bindings. And the DOM allows you to omit methods (and this omition is important), so we need to provide the ability to omit methods.

Web Components is not designed to be a performance-oriented standard. It's designed to help programmer with better boilerplate. You do not need web components to write web pages and the result will be the same.

The performance impact of Shadow DOM is way worse than an extra function call. And people are still using it. Why? Because the benefit of avoiding the hassle of tracking css styles and making sure they do not apply to elements that they shouldn't outweighs the performance impact. This is known as opportunity cost.

The opportunity cost for me to avoid some extra function calls is very low. I am not willing to write an extra piece of code to accomplish just that nor do those framework creators when they are already way fed up by hundreds of (if not thousands of) open issues.

Also, when I am writing a framework, I do not know if the user needs a feature. I have no choice but to implement them all. When the element is constructed, I do not know if the user needs mutation observation at a later point(it may depend on the content assigned to the element by Shadow Dom), but I have to define attributeChangedCallback now or I won't be able to provide data binding.

That has nothing to do with macros. The error you're getting is correct and precise, it is not obfuscated by macros.

This is not true. Normally, I must satisfy the function contract before I pass my variable to another function. This means that my error can never go somewhere else. This is one of the advantages of using a static typed language.

However, macros are able to write arbitrary code with no contract check. This pretty much like in Python or JavaScript you can generate code as string and then eval/exec them. You do not know if the code is correct or wrong until you invoke it. The error outside the macro may be shifted into macro definition due to the way macro works.

fn print_it<S: AsRef<str>>(s: S) {
    println!("{}", s.as_ref());
}

macro_rules! print_it {
    ($a:expr) => (println!("{}", $a.as_ref()));
}

fn main() {
    print_it("test");
    print_it(Ok("test"));

    print_it!("test");
    print_it!(
        //500 lines long
        //500 lines long
        //500 lines long
        //500 lines long
        //500 lines long
        Ok("test")
        //500 lines long
        //500 lines long
        //500 lines long
        //500 lines long
        //500 lines long
    );

}

rustc says:

error[E0277]: the trait bound `std::result::Result<&str, _>: std::convert::AsRef<str>` is not satisfied
  --> src/main.rs:11:5
   |
11 |     print_it(Ok("test"));
   |     ^^^^^^^^ the trait `std::convert::AsRef<str>` is not implemented for `std::result::Result<&str, _>`
   |
note: required by `print_it`
  --> src/main.rs:1:1
   |
1  | fn print_it<S: AsRef<str>>(s: S) {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0277]: `std::result::Result<&&str, &_>` doesn't implement `std::fmt::Display`
  --> src/main.rs:6:34
   |
6  |       ($a:expr) => (println!("{}", $a.as_ref()));
   |                                    ^^^^^^^^^^^ `std::result::Result<&&str, &_>` cannot be formatted with the default formatter
...
14 | /     print_it!(
15 | |         //500 lines long
16 | |         //500 lines long
17 | |         //500 lines long
...  |
25 | |         //500 lines long
26 | |     );
   | |______- in this macro invocation
   |
   = help: the trait `std::fmt::Display` is not implemented for `std::result::Result<&&str, &_>`
   = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
   = note: required by `std::fmt::Display::fmt`

error: aborting due to 2 previous errors

I do not think we can provide better error message under current macro system.

All of the Rust syntax is highlighted correctly, and the Rust syntax is 99% of the js_class! macro.

The custom_element attribute isn't syntax-highlighted either, it's the same situation.

99% of code you wrote is correct, it's the 1% that's driving rustc crazy.
Vim highlights #[] with purple and I think it looks pretty good.

Also, I'm not sure why you put in a superfluous constructor,

Because this is the reserved name for JavaScript constructor. Other names can be used for other purposes.

Except that the first version can omit methods, so in practice it will almost always be much shorter.

I am talking about the complexity to implement js_class! macro and #[custom_element].

and why you omitted the reference for the attribute version

I did put .... Because I do not need to implement them(I mean reference macro is implemented).
But for js_class!, I assume you still need to run them through TokenStream so you still need to write code take care of them.

Or why you changed struct to class?

Because it does make sense to separate methods from values in JavaScript. You can do this.value = 123 at any time.

Also, class definition is can fail(throws an exception) if your class name conflicts with another class, I am not sure how you recover from it if js_class! is invoked outside a function.

Can we do class extends HTMLElement and ask js_class! to return the anonymous class as Value?
That sounds reasonable to me. Though I'm not sure how useful it would be.

How about:

window().custom_elements().define("my-element", js_class!(class extends HtmlElement {...}));

Custom elements do not inherit from HTMLElement by default (and they should not inherit from HTMLElement by default).

Can you explain this to me? I think most of the time HtmlElement is what you want.

@koute
Copy link
Owner

koute commented Oct 28, 2018

A fair comparison (with all superfluous details stripped) would look like this:

js_class! {
    struct Foo(InputElement) extends HTMLInputElement {
        // ...
    }
}

custom_elements().define_extends("foo", &*Foo, "input");
#[custom_element(name = "foo", extends = "input", inherits = "HTMLInputElement")]
struct Foo(InputElement);

impl ICustomElement for Foo {
    // ...
}

Except that the first version can omit methods, so in practice it will almost always be much shorter.

Perhaps a better alternative would be a hybrid of those two, e.g.: (the exact syntax could probably be tweaked further)

js_class! {
    #[custom_element(name = "foo", extends = "input", inherits = "HTMLInputElement")]
    struct Foo(InputElement);

    impl ICustomElement for Foo {
        // ...
    }
}

Why? The first snippet requires some finicky custom parsing logic (as syn won't be able to parse it out-of-box), the second is not as flexible (so as @Pauan said we wouldn't be able to omit the methods if we wanted to), while the third one not only looks like completely valid Rust code, but is also parsable out-of-box by syn (so should be significantly simpler to implement than the first one) and still flexible (so technically we could implement it as a trait underneath, or do something else entirely).

I do not think we can provide better error message under current macro system.

With the macro_rules! you indeed can't really do much about the bad error messages, however with procedural macros you can. (See proc_macro::Diagnostic; it's still nightly-only though.)

Also, class definition is can fail(throws an exception) if your class name conflicts with another class, I am not sure how you recover from it if js_class! is invoked outside a function.

Do we need to recover from that? That sounds to me like a programmer error, in which case a panic! on Rust's side would be appropriate, so on the JS side a similar thing would also be acceptable, I think.

@Pauan
Copy link
Contributor

Pauan commented Oct 29, 2018

@futursolo The opportunity cost for me to avoid some extra function calls is very low. I am not willing to write an extra piece of code to accomplish just that nor do those framework creators when they are already way fed up by hundreds of (if not thousands of) open issues.

I'm not really sure what you're saying here. Stdweb provides low-level DOM bindings, the ability to omit methods is a part of custom elements, thus we must provide that functionality.

It doesn't have anything to do with you, or frameworks, or anything else. It's just the way that stdweb does things.

If you don't want to write the code, that's fine. I already said we can handle that in a different PR.

Also, when I am writing a framework, I do not know if the user needs a feature. I have no choice but to implement them all.

Your framework can provide the ability for users to specify what features they want. Just like how stdweb lets you specify that. There's many ways to write frameworks, you're only describing one specific way.

If your framework wants to always define all the methods, that's fine. There's nothing wrong with that.

But stdweb shouldn't require all methods to be defined. Stdweb needs to be flexible enough to be used for many different use cases, including people who want to write custom elements by hand (without a framework), or people who want to create a framework which doesn't require all methods to be defined.

Requiring all methods to be defined will affect everybody, which isn't good.

However, macros are able to write arbitrary code with no contract check. This pretty much like in Python or JavaScript you can generate code as string and then eval/exec them. You do not know if the code is correct or wrong until you invoke it. The error outside the macro may be shifted into macro definition due to the way macro works.

Yes of course you can do dynamic typing with macros... but that's not relevant to the js_class! macro, which is fully statically typed.

I do not think we can provide better error message under current macro system.

Once again... that's not an issue with the macro system, it's an issue with println!, because the arguments to println! are polymorphic (using a trait), so it has to infer the type.

With the print_it function you are providing type annotations so it's able to give good error messages.

But with the print_it! macro you don't provide any type annotations, so it has to infer the type, so of course the error messages are worse.

If you give type annotations within the macro, then it gives the same error messages with both the function and the macro.

Those issues do not apply to the js_class! macro, because it is providing the full static type of everything.

Please stop taking an example of one macro which has bad behavior and then claiming that all macros inherently have that bad behavior.

Especially because lazy_static (and others) have already proven that you can have good error messages with macros.

99% of code you wrote is correct, it's the 1% that's driving rustc crazy.

I'm not sure what you're talking about, we're discussing syntax highlighters, not rustc (rustc has no problem with custom syntax in macros).

In any case, I have no problem with using an #[extends = "Foo"] syntax rather than extends Foo, that's all bikeshedding stuff that can be easily discussed and changed.

And it has nothing to do with js_class! vs #[custom_element], because both approaches can use attributes, and both approaches can have custom syntax.

Because this is the reserved name for JavaScript constructor. Other names can be used for other purposes.

What I mean is, that the constructor was not necessary, but you added it in to the js_class! example (but not the #[custom_element] example).

I am talking about the complexity to implement js_class! macro and #[custom_element].

I am talking about the user facing API, and also the complexity to implement it. Both things have to be considered, not just the implementation complexity.

When we provide APIs in stdweb, we have to consider many things: the cost to implement it, the cost to maintain it, the cost to use it, the performance, how flexible it is, how much overlap it has with other features, how close it is to the JS API, how "Rust-y" it is, etc.

I've tried very hard to keep this conversation neutral and fact-based, without any emotion. I'm not sure why you seem to keep pushing so hard for one approach, and ignoring the pros and cons of other approaches. None of the approaches are perfect, there are complex and difficult trade-offs that we need to make.

If the issue is that you don't want to implement it, that's fine, I already said you don't need to implement it.

But in that case just say that you don't want to implement it. There's no need to keep arguing based solely upon how easy it is to implement it.

I assume you still need to run them through TokenStream so you still need to write code take care of them.

You need to run them through TokenStream for attributes as well, because attributes are proc macros.

Because it does make sense to separate methods from values in JavaScript. You can do this.value = 123 at any time.

I'm sorry, I don't understand this at all. We're discussing the user-facing syntax inside of the js_class! macro. Specifically whether to use struct or class to declare the class. That has nothing at all to do with methods or values.

Also, class definition is can fail(throws an exception) if your class name conflicts with another class, I am not sure how you recover from it if js_class! is invoked outside a function.

The js_class! macro always creates anonymous classes (as I showed in the desugaring earlier), so they won't conflict with any other classes, so that error will never occur.

How about:

window().custom_elements().define("my-element", js_class!(class extends HtmlElement {...}));

That is equivalent to this:

{
    js_class! { struct Foo(HtmlElement) extends HTMLElement { ... } }
    window().custom_elements().define("my-element", &*Foo);
}

Except that named classes are a lot easier to implement. Trying to support both named and unnamed classes is tricky and will require some careful thought.

Can you explain this to me? I think most of the time HtmlElement is what you want.

That is just how custom elements work:

https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_custom_elements

There are two types of custom elements:

  • Autonomous custom elements are standalone — they don't inherit from standard HTML elements. You use these on a page by literally writing them out as an HTML element. For example <popup-info>, or document.createElement("popup-info").

  • Customized built-in elements inherit from basic HTML elements. To create one of these, you have to specify which element they extend (as implied in the examples above), and they are used by writing out the basic element but specifying the name of the custom element in the is attribute (or property). For example <p is="word-count">, or document.createElement("p", { is: "word-count" }).

It is possible to create custom elements which don't inherit from any HTML elements. In fact that is the default, and you have to go out of your way to inherit from an HTML element.

@Pauan
Copy link
Contributor

Pauan commented Oct 29, 2018

@koute Perhaps a better alternative would be a hybrid of those two, e.g.: (the exact syntax could probably be tweaked further)

I think that's a good idea. I also very much agree that being able to parse it with syn is a big deal, so in that case we should try really hard to avoid all custom syntax.

However, I don't like the ICustomElement idea, because it implies that it's using traits (even though it's not). So I was thinking something like this:

js_class! {
    #[custom_element(name = "foo", extends = "input")]
    #[reference(extends = "HTMLInputElement")]
    struct Foo(InputElement);

    impl Foo {
        fn inited(&self) {
            // ...
        }

        // ...
    }
}

It will probably require a lot of tweaking and discussion (and prototype implementations) before we can figure out the best syntax. But I think this is a good start.

Do we need to recover from that? That sounds to me like a programmer error, in which case a panic! on Rust's side would be appropriate, so on the JS side a similar thing would also be acceptable, I think.

Thankfully js_class! creates anonymous JS classes, so that error can never occur.

@futursolo
Copy link
Contributor Author

Do we need to recover from that? That sounds to me like a programmer error, in which case a panic! on Rust's side would be appropriate, so on the JS side a similar thing would also be acceptable, I think.

I think it would be useful if someone wants to implement a polyfill.

@futursolo
Copy link
Contributor Author

I'm not really sure what you're saying here. Stdweb provides low-level DOM bindings, the ability to omit methods is a part of custom elements, thus we must provide that functionality.

Well, Window object will return all unused methods/values as undefined(thanks to JavaScript). I think this is a horrible feature.
But, "Stdweb provides low-level DOM bindings." The ability to treat undefined members as undefined is a part of the Window object, thus we must honour that.

Opportunity cost is an important factor involved in decision making. It helps decision maker to make better decisions. When people choose between manually check style classes/ids and shadow dom, each option involves a cost.

Manually checking style classes/ids is faster in terms of perforamance, but it can become very painful as the application becoming more complex. Shadow Dom can solve this problem with little hassle, but it impacts the performance.

Does this mean people who choose to use Shadow Dom does not care about performance? No. It is because the benefit of using shadow dom outweighs the performance gained by tracking styles manually.

This also applies to choosing which approach to use for custom elements.

It doesn't have anything to do with you, or frameworks, or anything else. It's just the way that stdweb does things.

It sounds like when making a web standard, W3C has nothing to do with browser vendors. I do not think this is how things works.

Requiring all methods to be defined will affect everybody, which isn't good.

What's the effect of defining all methods? 3~4 extra function calls.
How important are these function calls? I do not know. Probably my glorious senpai @Pauan can tell me.

I'm not sure why you seem to keep pushing so hard for one approach, and ignoring the pros and cons of other approaches.

Did I? Or, why you seem to keep pushing so hard for your approach? Will you say "I think that's a good idea." on something that you think is in fact horrible?

I've tried very hard to keep this conversation neutral and fact-based, without any emotion.

Please try harder. This is how the Internet works.

I am the one that really does not understand why are you so paranoid about the performance impact on function calls.

I'm sorry, I don't understand this at all. We're discussing the user-facing syntax inside of the js_class! macro. Specifically whether to use struct or class to declare the class. That has nothing at all to do with methods or values.

For me struct is a container for values. It has nothing to do with methods. You should not put methods in struct definition. And what's a struct associated with methods? A class. It is that simple.

That is just how custom elements work:

Well... I guess you didn't read that page all the way thought.

Let's have a look at an example of an autonomous custom element — <popup-info-box> (see a live example). This takes an image icon and a text string, and embeds the icon into the page. When the icon is focused, it displays the text in a pop up information box to provide further in-context information.

To begin with, the JavaScript file defines a class called PopUpInfo, which extends HTMLElement. Autonomous custom elements nearly always extend HTMLElement.

https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_custom_elements#Autonomous_custom_elements

The only real difference here is that our element is extending the HTMLUListElement interface, and not HTMLElement . So it has all the characteristics of a <ul> element with the functionality we define built on top, rather than being a standalone element. This is what makes it a customized built-in, rather than an autonomous element.

https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_custom_elements#Customized_built-in_elements

Also, Shadow Dom says it can be attached to Any autonomous custom element with a valid name. So I guess it needs to be at least Element.

Why I ditch window.custom_elements().define()/ window.custom_elements().define_extends()?

You should be able to learn what's the JavaScript Name from the struct since #[reference(instance_of = ...)] already stated. You should also be able to learn which built-in element to extends from which struct they extends. Probably something like

#[reference(tag_name = "textarea")]
// ...
pub struct TextareaElement;

.

Why I ditch js_class!?

I think we need some customization for custom elements standard to make it more polished and leverage the advantage of using a compiled, type-safe language. Not just simply make a Javascript class and toss them to window.custom_elements().define(). When these kinds of customization is in place, I do not think there's any key difference on defining them with js! or js_class!. If js_class! is not required in this implementation, then I am not interested in it.

@futursolo
Copy link
Contributor Author

futursolo commented Oct 29, 2018

Well, though I do not think that several extra function calls matters, I do think that omitting them improves readability.

How about something like

#[custom_element(...)]
impl ICustomElement for MyElement {}

?

So We move custom_element attribute to impl ICustomElement for MyElement. custom_element runs through the implementation, implement unused methods with unreachable!() on rust side and omit them them on its JavaScript class definition when creating a hidden define() method.

@futursolo
Copy link
Contributor Author

This also provides a macro-free version if somebody doesn't like macros. They can implement define()on their own.

@Pauan
Copy link
Contributor

Pauan commented Oct 30, 2018

@futursolo You are acting very unreasonable, so I refuse to comment anymore with you. Goodbye.

@futursolo
Copy link
Contributor Author

I only post "unreasonable" response to "very unreasonable" questions. To make me "very unreasonable", the questioner has to be "extremely unreasonable" theirselves.

I also initially refused to waste my time to honour concerns about "extra function calls" that would effect the overall design. I also said I am not interested in js_class!'s design and grammar. Do not discuss that with me. But you keep wasting my time and everybody else's time by overstating the problem "required", "important", and "MUST" with poor supporting points. This is total non-sense for me.

Don't forget to click "mute the thread" or "unsubscribe" to make sure you do not receive more "unreasonable" comments.

@koute
Copy link
Owner

koute commented Oct 30, 2018

Both of you, please, let's chill, okay? (: It's fine to have disagreements, but successively escalating the arguments is never productive as it usually turns into a shouting match.

@Pauan Thanks for all of your input so far.

@futursolo Thanks for your work on this PR. It almost looks good to me; I'd like to request two last changes:

  1. Get rid of the web-components example for now. We can add it later once it's a little bit more fleshed out.
  2. Please split the test_shadow_dom into two tests, one which will use assigned_nodes and the other which will use assigned_elements. Then please mark the currently not working one with the #[ignore] attribute without commenting it out.

Then we can merge this PR as-is and continue discussing the details on how to actually make the web components happen.

@futursolo
Copy link
Contributor Author

No problem.

While I don't regret on what I did, I legitimately feel sorry to everybody else who spent their time to read these non-sense.

I truly hope this kinds of drama never happen again.

@koute
Copy link
Owner

koute commented Nov 1, 2018

Thanks!

@koute koute merged commit 85e4e82 into koute:master Nov 1, 2018
@koute
Copy link
Owner

koute commented Nov 1, 2018

As far as adding support for actually defining web components goes, it would be nice to have something that is minimal, extensible, fully type checked and as declarative as possible. The best way to approach this is, I think, with a procedural macro.

E.g. it would be great to get something like this working:

web_component! {
    #[derive(Clone, Debug, PartialEq, Eq, ReferenceType)]
    #[reference(subclass_of(EventTarget, Node, Element, HtmlElement))]
    struct MyElement(HtmlElement);

    impl MyElement {
        fn constructor(&self) {
            // ...
        }

        fn connected(&self) {
            // ...
        }
    }
}

fn define_custom_elements() {
    // ...
    window().custom_elements().define("my-element", MyElement);
    // ...
}

Since you have to call window.customElements.define anyway we could probably afford a little bit of runtime trickery here to keep things nicer for the user.

Notice that in the snippet I've posted I didn't explicitly write extends = "HTMLElement" anywhere - we probably could figure this part out automatically for the user. That would require defining a new (private) trait, e.g. something like this:

trait TypeConstructor {
    fn type_constructor() -> Value;
}

and then making the normal ReferenceType derive macro for this code:

#[derive(Clone, Debug, PartialEq, Eq, ReferenceType)]
#[reference(instance_of = "HTMLElement")]
#[reference(subclass_of(EventTarget, Node, Element))]
pub struct HtmlElement( Reference );

emit something like this:

impl TypeConstructor for HtmlElement {
    fn type_constructor() -> Value {
        return js!( return HTMLElement; );
    }
}

And since when defining classes with class Foo extends Bar the Bar part can be an arbitrary expression we'll just be able to call the type_constructor() of the parent type inside of the struct MyElement(HtmlElement); when defining the class. And the class would be defined using a normal js! snippet generated by the procedural macro and triggered through another private trait when define is called, e.g. something like this:

trait CustomElement {
    fn define() -> Value;
}

impl CustomElement for MyElement {
    fn define() -> Value {
        return js!(
            return class MyElement extends @{HtmlElement::type_constructor()} {
                // ...
            };
        );
    }
}

The #[reference(instance_of = "MyElement")] is also not necessary since we can just take the name of the Rust structure verbatim and use that.

@futursolo Does this sound good to you?

@futursolo futursolo changed the title [WIP] Web Components Web Components Nov 1, 2018
@futursolo futursolo changed the title Web Components Web Components: Shadow Dom + Template Element Nov 1, 2018
@futursolo
Copy link
Contributor Author

Since this pull request is merged, can we move the discussion about the syntax for custom elements to an issue?

@koute
Copy link
Owner

koute commented Nov 1, 2018

Sure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants