Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to define APIs only for custom element authors #758

Closed
tkent-google opened this issue Jul 30, 2018 · 98 comments · Fixed by whatwg/html#4324
Closed

How to define APIs only for custom element authors #758

tkent-google opened this issue Jul 30, 2018 · 98 comments · Fixed by whatwg/html#4324

Comments

@tkent-google
Copy link
Contributor

The following issues need APIs which should be used by custom element authors and should not be used by custom element users.

At the March F2F we came up with one idea. However using ShadowRoot for such APIs looks very weird to me and I'd like to discuss it again.

Candidates:

  • ShadowRoot
    Pros: Usually only custom element implementation knows the instance of ShadowRoot which is used to implement the custom element. It's difficult for custom element users to get a ShadowRoot instance created by a custom element implementation [1]
    Cons: ShadowRoot is an interface for tree-encapsulation. Adding features unrelated to tree-encapsulation looks like a design defect. We should not make ShadowRoot a kitchen sink.
    Cons: Not all custom element implementations need ShadowRoot. For example, a checkbox custom element won't need ShadowRoot. Creating unnecessary ShadowRoot for such APIs is not reasonable.
    Cons: If a custom element implementation uses no ShadowRoot, a custom element user can call element.attachShadow() to get a ShadowRoot instance, and thus get access to these private APIs.

  • new Something(customElement)
    It throws if users try to create it for the same element twice. It throws if the constructor is called with non-custom elements. The interface Something is called as ElementStates in Custom pseudo-classes for host elements via shadow roots (:state) #738, and called as HTMLElementPrimitives in Need callback for form submit data #187.
    Pros: ShadowRoot won't have unrelated APIs.
    Pros: Usually only custom element implementation knows the instance of Something. It's difficult for custom element users to get a Something instance created by a custom element implementation [1]
    Cons: If a custom element implementation uses no Something, a custom element user can call new Something(element) successfully to get a Something instance.

  • Deliver a Something instance by a custom element callback
    See Need callback for form submit data #187 (comment) for more details.
    Pros: ShadowRoot won't have unrelated APIs.
    Pros: Custom element users can not create a Something instance unlike other two candidates.
    Pros: It's difficult for custom element users to get a Something instance delivered to a custom element [1]
    Cons: UA implementation would need larger code for a new callback, compared to the other two candidates.

IMO, the second one or the third one is much better than the first one.

@annevk @domenic @rniwa @trusktr What do you think?

[1] If a custom element implementation stores a ShadowRoot / Something instance to this.foo_, a custom element user can get the instance by accessing yourElement.foo_. There are some techniques to avoid such casual access.

@caridy
Copy link

caridy commented Jul 30, 2018

I have a similar realization the other day when polyfilling shadowRoot.ariaLabel and co., which can fall into the same category. shadowRoot becoming a kitchen sink will be unfortunately, but considering that WC APIs are very low level, it might be ok.

@annevk
Copy link
Collaborator

annevk commented Jul 31, 2018

I don't quite understand why delivering Something via a callback is more expensive than new Something(). (A way to avoid the "cons" of new Something() is perhaps to allow declaration ahead of time that the custom element won't use it, or indeed, require explicit opt-in that it's going to be used.)

(Another "cons" of ShadowRoot is that it's not limited to custom elements.)

@alice
Copy link
Member

alice commented Aug 2, 2018

I'm writing up the properties @caridy mentioned.

The design problem we're trying to solve there is:

  • We need a way to set non-reflected versions of the ARIA IDL attributes which will change dynamically
    • e.g. ariaChecked, which represents the current "checked" state
  • These versions should be shadowed by the reflected version
    • e.g. if I set ariaChecked on the host element, as well as the shadow root, the version on the host element should override whatever was set on the shadow root - but then if I delete the ariaChecked property on the host, it should revert to using the version on the shadow root

ShadowRoot really seems like a good fit for this problem in some ways, since it's analogous to using :host in a <style> element within the shadow root - but unlike style, the properties on the host element can only be set directly on the host element (i.e. can not be defined anywhere but directly on the affected element), so there's no logical place to put the :host equivalent.

It also means that you can encapsulate semantic information about the host element without needing to register a custom element.

However, I can imagine asking the question "why can't I set shadowed versions of any element property on the shadow root?" - which definitely sets us on a road to a kitchen sink scenario.

Also, if your custom element doesn't otherwise need a shadow root, it seems like a shame to need to attach one for this purpose.

We could theoretically create a new type of object to hold semantic properties, analogous to a constructable stylesheet, but unlike constructable stylesheets there's no precedent for the type of object this would need to be - it's really just a simple, small map of properties.

Does anyone have any ideas for alternatives to using ShadowRoot here?

@annevk
Copy link
Collaborator

annevk commented Aug 2, 2018

@alice @tkent-google's post has a number of alternatives, no? We'd need a pick a better name than Something, but otherwise one of those approaches would work I think.

@alice
Copy link
Member

alice commented Aug 2, 2018

@annevk I should have addressed those better, sorry!

Here are my concerns with the those approaches (I count only two, really - the first alternative is just to use ShadowRoot):

  • Either way, we'll have to create a new type which is simply an empty object with AccessibilityRole and AriaAttributes mixed in, which is kind of awkward.
  • The "con" noted for the new Something(element) option seems like a deal breaker to me (that anyone can construct one for an element which doesn't already have one) - it's the opposite of encapsulation, which is what we're aiming for. (Presumably constructing a Something for a built-in element would be forbidden). Do authors need to construct a new Something defensively whenever they create a custom element, for all possible values of Something (if many APIs use this strategy)?
  • The lifecycle callback option is probably the one I would be least uncomfortable with, but I think it implies having a single object named something like ElementConfiguration which can grow to hold all of these various objects (ElementStates, HTMLElementPrimitives, etc.) which is passed in in a single lifecycle event. Otherwise, it seems like either each new API has to create a new lifecycle event (?) or else the createdCallback ends up with a mess of arguments.

@trusktr
Copy link
Contributor

trusktr commented Aug 2, 2018

but considering that WC APIs are very low level, it might be ok.

That may be an overstatement. Web devs want to make components, so if they move off React or Vue (or etc) to Web Components, they will use ShadowDOM. It's not very low level, it's a normal part of organizing web UI. Any serious web developer should be expected to organize code with components of nested trees; it's standard.

I'm in favor not to use it as a kitchen sink for component parts. Rather, ShadowDOM is it self a component part, and should keep it's concerns specific. Other parts can have other jobs.


@tkent-google About new Something (element), what's that idea? Is that a suggestion for an entity-component system?

@alice
Copy link
Member

alice commented Aug 6, 2018

@annevk and @tkent-google I just raised WICG/aom#127 to try and avoid further rabbit-holing here on this specific issue, but I'd appreciate your thoughts over there if you have time!

@alice
Copy link
Member

alice commented Aug 6, 2018

One benefit of the createdCallback (presumably?) option is that, if we do combine all possible settings into something like ElementConfiguration, the API gives you an opportunity to learn by exploration of that object.

With new MyThing(element), authors would have to learn about each MyThing separately (although obviously good docs can help with this).

Also, if new MyThings keep being added, custom elements which were written before they existed run the risk of having MyThings being defined by the page instead. This may not be a concern, but it seems awkward to me.

@trusktr
Copy link
Contributor

trusktr commented Aug 13, 2018

@tkent-google About new Something (element), what's that idea?

Oops, nevermind, it was based on the idea of new ElementStates(this) which you linked to. Sorry for the noise.

Deliver a Something instance by a custom element callback

I think that's my favorite idea.

I added an example using that idea here: #738 (comment)

To avoid enabling all features (and avoid runtime cost) maybe they can be opt-in? f.e.,

import Privates from './privates-helper'
const _ = new Privates

class MyEl extends HTMLElement {
  useFeatures(features) {
    _(this).cssStates = features.use('cssstates')
    // or
    _(this).cssStates = features.cssStates()
  }

  connectedCallback() {
    // blink every second
    setInterval(() => {
      _(this).cssStates.has('enabled') ?
        _(this).cssStates.remove('enabled') :
        _(this).cssStates.add('enabled')
    }, 1000)
  }
}

We could achieve this example with CSS, but this example is similar to how a div element doesn't add a hover class for you, but instead a :hover state, so it doesn't interfere with the div user's class list.

@hayatoito
Copy link
Contributor

hayatoito commented Aug 14, 2018

@tkent-google,

Deliver a Something instance by a custom element callback

It looks this idea is the most favored. Something might become yet another kitchen-sink, but I think that would be much better than using ShadowRoot here.

Could you have a chance to make the idea more concrete one or prototype?
Several new features are waiting for this idea, I guess.

@tkent-google
Copy link
Contributor Author

Many people are favor of the third one. Let's proceed with the third one if no one has a strong opinion against it.

We need to define the followings:

Interface name

HTMLElementPrimitives (drop HTML?), ElementSemantics, etc.
Please reply your ideas.

Callback

First I thought we could add a Something argument to connectedCallback, but I found that custom element implementations needed to call a protected API before connecting to a document tree in Form Participation API. So we should introduce new callback.

My current proposal is createdCallback, which is called just after upgrade, maybe before attributeChangedCallback.

@tkent-google
Copy link
Contributor Author

@annevk

I don't quite understand why delivering Something via a callback is more expensive than new Something().

I guess adding a new custom element callback requires larger code in UA than allowing new Something(element) just once. We already have four callbacks, and adding another would not be a big issue.

@annevk
Copy link
Collaborator

annevk commented Aug 15, 2018

Another name idea: ElementInternals (after internal slots). Without HTML seems reasonable as we'd reuse this if we ever added custom elements in other namespaces. createdCallback also seems reasonable, though a bit unfortunate we then have both that and a constructor.

@caridy
Copy link

caridy commented Aug 15, 2018

ElementInternals is definitely better than ElementSemantics, +1 on that.

@alice
Copy link
Member

alice commented Aug 15, 2018

Would we put all of the properties on what I've called ElementSemantics on ElementInternals, or would we have a semantics object hanging off ElementInternals like I proposed (I referred to ElementInternals as ElementConfiguration as a placeholder in that doc, and in my comments above)?

@caridy
Copy link

caridy commented Aug 15, 2018

@alice I think a flatten structure is just fine. HTMLElement itself is mostly flatten anyways.

@trusktr
Copy link
Contributor

trusktr commented Aug 20, 2018

Extending from option 3, could the following help with performance?

Option 4: static list of features.

What if elements could specify specific features to use in a static prop, so the engine doesn't unnecessarily have to pass every single feature into a callback even if an element won't use each feature?

F.e.

class Foo extends HTMLElement {
  static get observedAttributes() { ... }
  static get features() { return [ ... ] }
}

Then the features could do what they want: specify new callbacks, or provide certain instance props, etc.

Either it would be up to the feature how it is exposed, or perhaps they all get injected into a standard callback like option 3's createdCallback (though I think a different name would be better because we already have constructor).

As for the static list of features, if they were strings,

  static get features() { return [ 'builtin-feature', 'user-feature' ] }

it would be easiest, but with the problem of name clashing (suppose we let anyone provide features, not built-in features):

  static get features() { return [ BuiltinFeature, UserFeature ] }

References would avoid name clashing, and not require a registry:

But then, this seems like mixins!

Option 5: class-factory mixins

Could mixins do the trick?

class Foo extends BuiltinFeature( UserFeature( HTMLElement ) ) {

As a real-world example, SkateJS is a web component library whose features are consumable as mixins, making them easy to mix and match.

@tkent-google
Copy link
Contributor Author

ElementInternals sounds good. If no one objects it, let's adopt it.

@alice @caridy I also suppose flatten structure. ElementInternals interface has attributes/operations for various features such as accessibility, form controls, ...

@trusktr I'm not sure I understand what you wrote correctly. Probably such feature list isn't necessary because we assume a single ElementInternals instance handles all features for the associated element?

Re: callback name
createdCallback represents the timing when it is called, and doesn't represent the purpose. We may give more descriptive name like elementInternalsCreated receiveElementInternals.

@trusktr
Copy link
Contributor

trusktr commented Sep 2, 2018

@tkent-google

because we assume a single ElementInternals instance handles all features for the associated element?

That's what I was trying to avoid with the idea of a list or mixins, because otherwise it means ElementInternals will include all possible features even if the features are not going to be used.


In Option 5 above, class-factory mixins provide a way to opt-in to using specific features (and avoid resource waste by creating the feature instances only when needed):

import UserFeature from 'npm-package'
const {BuiltinFeature1, BuiltinFeature2} = customElements.elementFeatures

class MyEl extends BuiltinFeature1( BuiltinFeature2( UserFeature( HTMLElement ) ) ) {
  // ...
}

customElements.define('my-el', MyEl)

In that sample, the class will have only the features it has specified.


If the list gets long, then

const Features = BuiltinFeature1( BuiltinFeature2( UserFeature( HTMLElement ) ) )

class MyEl extends Features {
  // ...
}

@trusktr
Copy link
Contributor

trusktr commented Sep 2, 2018

@alice An example of option 5 for element semantics would be like

// ...
const {withSemantics} = customElements.elementFeatures

class MyEl extends withSemantics( HTMLElement ) {
  // ...
  receiveSemantics( semantics ) {
    const privates = elementPrivates.get(this);
    privates.semantics = semantics;
  }
}

customElements.define('my-el', MyEl)

where in this example, the receiveSemantics callback will be called by the withSemantics mixin implementation.

The withSemantics implementation might call the receiveSemantics method in its constructor, for example.

The implementation of withSemantics might look like the following (except it may not be JavaScript):

window.customElements.elementFeatures.withSemantics = function withSemantics(Base) {
  return class WithSemantics extends Base {
    constructor() {
      super()
      if (typeof this.receiveSemantics === 'function') {
        const semantics = ___; // create the semantics for `this` element
        this.receiveSemantics( semantics )
      }
    }
  }
}

It could also be placed on window.elementFeatures, but that creates a new window global. Seems like customElements would be a good fit for this.

@trusktr
Copy link
Contributor

trusktr commented Sep 2, 2018

Another example could be observing children. A mixin could make this an opt-in feature:

const {withChildren} = customElements.elementFeatures

class MyEl extends withChildren( HTMLElement ) {

  // This callback is provided by the `withChildren` mixin.
  childrenChangedCallback() {

    // Work with children here, don't worry about
    // if children exist in `connectedCallback`.

    // Not only will this fire whenever children change, but
    // also once after `connectedCallback` has been called.

    // Maybe children here are guaranteed to be already
    // upgraded if they are custom elements?

  }

}

@tkent-google
Copy link
Contributor Author

@trusktr, with your ideas, can we realize APIs which are available only for custom element authors?

@trusktr
Copy link
Contributor

trusktr commented Sep 15, 2018

can we realize APIs which are available only for custom element authors

Are you wondering if we can allow the mixins to be used only with Element classes? If so, then yeah we can:

The mixin could check the base class to make sure it is an Element class, f.e., pseudo code of the native code in JS:

function withAwesomeness(Base = HTMLElement) {
  if (!extendsFrom(Base, Element)) throw new TypeError('mixin can only be used on classes that extend from Element')

  // continue, return class extends Base...
}

Or did you mean something else?

aarongable pushed a commit to chromium/chromium that referenced this issue Nov 19, 2018
…chInternals().

- Introduce empty ElementInternals interface
- Add HTMLElement.prototype.attachInternals()
- Add a flag to ElementRareData about ElementInternals attachment
- Promote "ElementInternals" runtime flag to "test"

All the change is behind the runtime flag.
Some test cases in external/wpt/custom-elements/CustomElementRegistry.html
fail by this CL because this CL enables GET() operation for "disabledFeatures",
and these test cases doesn't expect it.  We should update the testcases later.


Bug: 905922
Bug: WICG/webcomponents#758
Change-Id: I7b3dbfd1d422c7eae5c7a6e01ddea50a00134a6e
Reviewed-on: https://chromium-review.googlesource.com/c/1341734
Reviewed-by: Hayato Ito <[email protected]>
Commit-Queue: Kent Tamura <[email protected]>
Cr-Commit-Position: refs/heads/master@{#609231}
@domenic
Copy link
Collaborator

domenic commented Nov 19, 2018

I don't think we should be contemplating such unusual patterns for the web platform; we should stick with normal class hierarchies.

@trusktr
Copy link
Contributor

trusktr commented Jan 29, 2019

@tkent-google

I can't imagine how to implement it in Blink with V8 though I'm a Chrome engineer.

Isn't there a feature of Chrome where new features can be implemented in JavaScript before they are (if ever) converted to C/C++? I've seen the debugger pause on such builtin features before. And, I'm just guessing, but if they can be implemented in JS, could the same mechanics be used on the native side?

Okay, assuming implementation details work out fine, is there anything bad about the pattern itself?

Why can't WebIDL define such a sort of feature? Is it because it is language specific (JS has this ability)?

In that sense, then aren't classes also language-specific and thus we shouldn't use them (f.e. Custom Elements)?

@trusktr
Copy link
Contributor

trusktr commented Jan 29, 2019

we should stick with normal class hierarchies.

What constitutes a normal hierarchy?

Can't we say that after we've mixed a mixin class into a new class definition, that the new definition is now a normal hierarchy? It works just like other hierarchies, plus we have things like Symbol.hasInstace to make instanceof work fine, etc.

By the way, I'm not absolutely married to the idea, but so far I like it. I'm curious to know why it would be a bad pattern compared to others.

aarongable pushed a commit to chromium/chromium that referenced this issue Jan 30, 2019
- Stop to make it mandatory
  It didn't make much sense because web developers can provide empty callbacks.

- Add 'mode' argument
  It is 'restore' if the callback called for state restore by UA.
  It will be 'autocomplete' if UA's autofilling feature calls the callback.

Bug: 905922
Bug: WICG/webcomponents#758
Change-Id: I747eb3f1cba97bd9d0b62ed18fd0504fae7250f9
Reviewed-on: https://chromium-review.googlesource.com/c/1438794
Commit-Queue: Kent Tamura <[email protected]>
Auto-Submit: Kent Tamura <[email protected]>
Reviewed-by: Hayato Ito <[email protected]>
Cr-Commit-Position: refs/heads/master@{#627355}
@trusktr
Copy link
Contributor

trusktr commented Feb 1, 2019

Hell you all, I just want to reiterate that I feel passing options like needsInternal or default styles (re: #468) to customElements.define seems out of place because:

  • authors define classes.
  • authors can delegate the use of customElements.define to end users so that end users can name the elements as they wish.
    • it wouldn't make sense to require all end users to know the specifics of the class in order to pass those specifics into the customElements.define call.
    • This is what class is for! Let the author define the class specifics on the class, and let the end user use the element without worrying about the class specifics.

For this reason I think that mechanisms like @decorators, static props, and Mixins(). are better. The author can decide what APIs a class needs, what styles it has, what behaviors it has, etc.

The end user just needs to define the element, and use it.

So in my opinion, authors of the custom elements should use class infrastructure to its fullest rather than putting parts of the class into customElements.define calls.

Author:

import styled from 'lib1'
import withRender from 'lib2'
import html from 'lib3'
import attribute from 'lib5'
// or const {attribute} = CustomElements // builtin
const [One, Two, Three] = window.ElementFeatures // builtin

@styled({
  foo: {
    color: 'skyblue'
  }
})
class AwesomeElement extends withRender(HTMLElement) {
  static elementInternals = [One, Two, Three]
  static userAgentStyle = new window.CSSStyleSheet( ... )
  
  @attribute('material-color', Color)
  materialColor = new Color('red')

  render() {
    return html`<div class="${this.classes.foo}"></div>`
  }
}

End user:

import AwesomeElement from 'awesome-element'
customElements.define('awesome-el', AwesomeElement)
document.body.innerHTML = `<awesome-el material-color="pink"></awesome-el>`

But then again, the author could abstract it away.

Author:

// adding onto the same file:
export const define = name => {
  customElements.define(name, AwesomeElement, {
    userAgentStyle: new window.CSSStyleSheet( ... ),
    elementInternals: [One, Two, Three]
  })
}

End user:

import define from 'awesome-element'
define('awesome-el')
document.body.innerHTML = `<awesome-el></awesome-el>`

I guess abstracting works, but now its an extra step.

If I'm already using class to define my class, why not just use class to define everything about my class? 😃

@trusktr
Copy link
Contributor

trusktr commented Feb 1, 2019

The only reason why customElements.define even needs to accept a name is presumably to allow the end user to customize this important aspect of an element. Otherwise, all we need is

class MyEl extends HTMLElement {
  static get elementName() { return "my-el" }
}

customElements.define(MyEl)

So unless it is critical for an end user to change something, then I feel it doesn't need to go into customElements.define because we already have class.

@trusktr
Copy link
Contributor

trusktr commented Feb 2, 2019

But then again, isn't an end user able to change stuff by extending from a class?

F.e., a class can easily be extended by an end user and a static elementName value overridden. It just seems like if we're using class, maybe we should be sticking class specifics inside the class definition rather than passing class specifics to an API outside of the class definition.

class MyEl extends HTMLElement {
  static get elementName() { return "my-el" }
}

class NewEl extends MyEl {
  static get elementName() { return "new-el" }
}

customElements.define(MyEl) // defines <my-el> element
customElements.define(NewEl) // defines <new-el> element

What would be the downside of having class-specifics inside the class definitions instead passing the specifics to customElements.define?

I can see passing the name into customElements.define makes it easy to do a simple re-name without having to write an additional class ... extends ... { static elementName = '...' }, but for other stuff like styles and API features, I think the class definition is there for that!

@tkent-google
Copy link
Contributor Author

As for ElementInternals, we made consensus around TPAC 2018. Though I haven't received any feedbacks from TAG, a specification PR was already approved, and we'll merge it with a PR for form-associated custom element.

tkent-google added a commit to tkent-google/dom that referenced this issue May 13, 2019
tkent-google added a commit to tkent-google/dom that referenced this issue May 13, 2019
domenic pushed a commit to whatwg/html that referenced this issue May 16, 2019
This provides the ElementInternals interface, which can be obtained for
custom elements via the element.attachInternals() method. For now
ElementInternals is empty, but it will gain members in #4383.

This also adds the ability for custom elements to set the
disabledFeatures static property, to disable element internals and
shadow DOM. Some DOM-side infrastructure work there is necessary in
whatwg/dom#760.

Tests:
- web-platform-tests/wpt#15123
- web-platform-tests/wpt#15516
- web-platform-tests/wpt#16853

Fixes WICG/webcomponents#758.
domenic pushed a commit to tkent-google/dom that referenced this issue May 16, 2019
domenic pushed a commit to whatwg/dom that referenced this issue May 16, 2019
@justinfagnani
Copy link
Contributor

Does anyone have an ElementInternals explainer?

@tkent-google
Copy link
Contributor Author

@justinfagnani AFAIK, Form Participation API Explained is the only document other than the HTML specification.

@annevk
Copy link
Collaborator

annevk commented Jun 28, 2019

In due course we should probably add an introduction section to the HTML Standard.

@frank-dspeed
Copy link

frank-dspeed commented Dec 24, 2019

Maybe we should simply point out that customElements are not a magic thing without the define stuff and all that they are

//CustomElement based on HTMLUnknownElement
const myCustomElement = document.createElement('my-element')
//Then apply some lhooks or what ever to it
myCustomElement.connectedCallback = ()=> {
  this.innerHTML = '<p>Iam a Complex Application </p>'
}
document.body.append(myCustomElement)

// We can also use existing HTML Elements
const MYCustomElment = document.getElementById('my-element')
MYCustomElment.connectedCallback = () => {
  this.innerHTML = '<p>Iam a Complex Application </p>'
}
document.body.append(myCustomElement)

Now the leaved out question is how does get connectedCallback get called ? you can do it manual or via mutationObserve and register it then you can also attach more behavior or bindings its a customElement so your free

as long as you understand a CustomElement is a modifyed version of a HTMLElement Instance.

@trusktr
Copy link
Contributor

trusktr commented Nov 15, 2023

I wrote a new issue describing how an internals callback would be a lot better than the current this.attachInternals(), and also proposed some ideas that could be doable when decorators land in browsers. These ideas keep internals fully protected:

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

Successfully merging a pull request may close this issue.