Skip to content

Conversation

EmmanuelOga
Copy link
Contributor

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.

…ined"

Currently `register` returns the result of `customElements.define`, which happens to be undefined.

While it is possible to get the just defined tab right away, it would make sense to return the newly create HtmlElement class instead.

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

vs

```js
register(Component, 'element-tag');
const klass = customElements.get('element-tag');
```
@EmmanuelOga
Copy link
Contributor Author

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
Copy link
Member

register is meant to closely resemble define which returns nothing.

I think this is better left to user implementations.

@EmmanuelOga
Copy link
Contributor Author

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
Copy link
Member

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.

@rschristian rschristian reopened this Sep 6, 2025
@rschristian
Copy link
Member

Year late but you convinced me

@rschristian rschristian merged commit 4e2c16b into preactjs:master Sep 6, 2025
1 check passed
@rschristian rschristian mentioned this pull request Sep 6, 2025
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.

2 participants