preactjs / preact-custom-element

Wrap your component up as a custom element
MIT License
360 stars 52 forks source link

Return the newly created HtmlElement class, as opposed to just "undef… #91

Closed EmmanuelOga closed 3 months ago

EmmanuelOga commented 3 months ago

It would make sense for register to return the newly created HtmlElement class instead of just undefined.

const klass = register(Component, 'element-tag');

vs

register(Component, 'element-tag');
const klass = customElements.get('element-tag');

Just a little more ergonomic for those wanting to get a reference to the newly created class.

EmmanuelOga commented 3 months ago

Additionally, it would be good to allow passing all the possible options for attachShadow, since right now only {'mode'} is configurable. Perhaps on a separate PR.

rschristian commented 3 months ago

register is meant to closely resemble define which returns nothing.

I think this is better left to user implementations.

EmmanuelOga commented 3 months ago

Up to you, but in practice the code is so much better when the class is returned. In my use case, I just want to add some methods to the prototype:

  const ELEMENT_API = { method1() {}, method2() {}, ...};

So it ends up being like this:

  const tag = "my-tag-name";
  register(MyComp, tag, ["attr1", "attr2"]);
  Object.assign(window.customElements.get(tag)?.prototype, ELEMENT_API);

Note the unnecessary use of ?.: without which most linters would complain.

With this PR:

  const klass = register(MyComp, "my-compl", ["attr1", "attr2"]);
  Object.assign(klass.prototype, ELEMENT_API);

I find myself adding methods to the custom element pretty often, so I think this could be a good change for anyone needing to do that. Users that don't need the return value can just ignore it.

rschristian commented 3 months ago

An extra loc is very serviceable, we'll keep this as-is for now.

Thanks for the proposal though, and if you did want to add support for all .attachShadow options that'd be much appreciated.