hybridsjs/hybrids

Typings issue with manual rendering

Closed this issue ยท 3 comments

Hi, first of all let me thank you for this great library! ๐Ÿ˜„ I love the concise syntax it promotes and look forward to using it a lot. Thanks for all the work you did to publish it and make it usable for us.

I stumbled upon an issue specifically with using TypeScript with hybrids. I am trying to get a reference to an Element part of a component's template. I need this reference because I am using another library which requires an Element reference as a parameter. It is an imperative library that binds an event listener on a button passed as parameter and opens a "popover" screen when the button is clicked.

I followed the documentation reference internals to get a hold on an element in my component. Unfortunately, TypeScript does not let me call host.render() or host.content() because it says those functions don't exist on HTMLElement & MyComponentInterface.

I've set up a Code Sandbox to reproduce the issue here: https://codesandbox.io/s/x98p5
In this repository, it works because of the two //@ts-ignore comments. If you remove the comments, TypeScript will not let it compile.
Since it works at runtime, I believe the issue is entirely related to typings. If you are able to reproduce this, let me know if I can help fixing those types. I could open a pull request.

Have a good day !

Hi @Hyzual! Thanks for such nice words about the project. I am eager to help if you encounter any other problems.

Fortunately, I am currently using at work hybrids in TypeScript, so I've found many bugs in typings and I fixed them already. You can easily fix your problem by adding an explicit definition of the content property (for the render would be the same), if you use the render factory implicitly. However, you could ask why, as it works well without that?

There are two reasons. The first is the TS limitation itself. It does not support generic values (more here: microsoft/TypeScript#17574), so we are required to create an interface instead of taking it from the hybrids object (we cannot pass the dynamic type itself as an argument of the function within that definition). Ultimately, I would like to have only hybrids definition without an additional interface (as it usually is just redundant).

The second reason is the dynamic behavior of the property definition. You are not forced to use render factory, when defining render or content property. The factory is used only if the property definition is a function (look at the translation rules). Because of that, the Hybrids or TaggedHybrids type cannot assume that your content or render will use that factory for sure, so it is defined in the way, that if you don't use those in other properties, it allows to set them as a function. However, if you need that property in another call, you must set it explicitly.

For the render factory, the interface property should be defined as () => HTMLElement, and adding that field fixes your issue:

interface ComponentWithRef {
  button: HTMLButtonElement | null;
  content: () => HTMLElement;
}

I did some other small fixes (use "as" in querySelector() as it returns Element type instead). You can check out the full example here (your forked example): https://codesandbox.io/s/hybrids-query-content-in-typescript-issue-forked-60gh3?file=/src/index.ts:87-183

EDIT: PS. I now that TS docs are not the best, just one document, so I think I will add extending docs for more TS examples to my todo list :)

Thank you very much for the quick and detailed answer! I'll be sure to take a look at it, probably not before Monday.

Hi, I have finally got some time to test your solution and it did work wonderfully, thank you very much ๐Ÿ™ !
If you need my help, I could try submitting a pull request for the typescript docs to help document that.