Add a customElementRoot prop for registered Custom Elements#92
Open
EmmanuelOga wants to merge 1 commit intopreactjs:masterfrom
Open
Add a customElementRoot prop for registered Custom Elements#92EmmanuelOga wants to merge 1 commit intopreactjs:masterfrom
customElementRoot prop for registered Custom Elements#92EmmanuelOga wants to merge 1 commit intopreactjs:masterfrom
Conversation
Avoids the need to setup a Ref and query its parentElement to gain access to the root of the Custom Element.
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 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"]); |
Contributor
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
I propose adding the root element as prop. This is a very minimal change that would make the API a lot cleaner:
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.