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

Add a customElementRoot prop for registered Custom Elements #92

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

EmmanuelOga
Copy link

@EmmanuelOga EmmanuelOga commented Jul 24, 2024

NOTE: this a "show don't tell" PR, and I'm guessing it may require changes to other parts of the library, but before adding docs or tests, I want to test the waters.

Working with custom elements, it is almost always necessary (except for the simplest of components) to get a hold of the root of the element, for instance to register event handlers to communicate with other parts of the DOM.

So, a Web component needs to do something like this:

function MyComponent(props: { /* ... */ }) {
  const self = useRef<HTMLDivElement>();

  useEffect(() => {
    const root = self.current?.parentElement as HTMLElement;
    if (!root) return;
    const handler = (ev: Event) => { /* ... */ };
    root.addEventListener("my-event", handler);
    return () => root.removeEventListener("my-event", handler);
  });

  return <div ref={self as Ref<HTMLDivElement>}>...content...</div>;
}

register(MyComponent, "my-component", ["prop1", "prop2"]);

I propose adding the root element as prop. This is a very minimal change that would make the API a lot cleaner:

function MyComponent(props: Props & { customElementRoot: HTMLElement }) {
  useEffect(() => {
    const handler = (ev: Event) => { /* ... */ };
    customElementRoot.addEventListener("my-event", handler);
    return () => customElementRoot.removeEventListener("my-event", handler);
  });

  return <div>...content...</div>;
}

register(MyComponent, "my-component", ["prop1", "prop2"]);

This is just one option, another one could be to have some sort of hook to pick up the root element, but I think that would be a bit more complicated to setup. I like this change because is very minimal and should not even conflict with people that already have that prop name (unlikely as that might be).

Also, since the Component function is always called after the DOM element has been instantiated, it makes sense to make it available to the component without needing to use a Ref.

Avoids the need to setup a Ref and query its parentElement to gain access to the root of the Custom Element.
@developit
Copy link
Member

Neat idea! Another option that would be similar but avoid having to inject into props would be to expose it via context with a $-prefixed name so it's super unlikely to clash with an existing context key and nicely marks the value as being an element. In Preact, context is already passed as the second argument to function components (and the third argument to class component render methods):

function MyComponent(props: Props, { $host: HTMLElement }) {
  useEffect(() => {
    const handler = (ev: Event) => { /* ... */ };
    $host.addEventListener("my-event", handler);
    return () => $host.removeEventListener("my-event", handler);
  });

  return <div>...content...</div>;
}

register(MyComponent, "my-component", ["prop1", "prop2"]);

@EmmanuelOga
Copy link
Author

That sounds neat! But I'm not sure how it would be implemented 😅

If I get it right, the context here is retrieved from a parent, so I'd need to check that the context there is null/undefined?

const context = event.detail.context ?? { $host: this._root } 

... something like that? I haven't needed slots so far so I'm not sure if that is correct ... 😬

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