Type Definitions for React CustomEvents

[AykutSarac]

Describe the bug
EventHandler properties are not shown for CustomEvents on React & TypeScript

To Reproduce
Steps to reproduce the behavior:

1. Create <BlSelect /> component
2. Add onSelect
3. On onSelect={e => ...} you won't be able to see custom properties of Event Handler referenced as "e" however it can be accessed

Versions
Baklava v2.0.0-beta.38
React 18.2.0

[AykutSarac]

I'm currently looking into it. From what I've seen so far is ,each @property definition should be added into the CustomEvent interface's parameter.

EDIT: welp, not exactly what I expected. I'll see if I could create a fix for that and open a PR.

[AykutSarac]

Could it be because the dispatcher function takes an interface <T> for event decleration however it was not referenced while emitting? https://github.com/Trendyol/baklava/blob/next/src/utilities/event.ts#L30

So in this case it should be updated with the following?

export function event<T>(customName?: string) {
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  return (protoOrDescriptor: any, name: string): any => {
    const descriptor = {
      get(this: HTMLElement) {
        return dispatcher<T>(this, customName || name);
      },
      enumerable: true,
      configurable: true,
    };

    Object.defineProperty(protoOrDescriptor, name, descriptor);
  };
}

and while declaring events
from this:
@event('bl-change') private onChange: EventDispatcher<string>;
to this:
@event<string>('bl-change') private onChange: EventDispatcher<string>;

Haven't tested yet so not sure if it will work though, just an idea.

[muratcorlu]

@AykutSarac is this issue still valid?

[AykutSarac]

Yes

[muratcorlu]

Apparently we can set event types like defined here: https://github.com/lit/lit/tree/main/packages/labs/react#typescript

I'll try to prioritize this.

[AykutSarac]

Apparently we can set event types like defined here: https://github.com/lit/lit/tree/main/packages/labs/react#typescript

I'll try to prioritize this.

That's awesome, let me know if anything that I can contribute!

[muratcorlu]

I quickly checked the complexity. Here is what I could find:

For having simple event types, it looks easy to set them. Our component analyzer already parses event types. An example from Textarea component¹:

{
  "kind": "class",
  "description": "",
  "name": "BlTextarea",
  "events": [
    {
      "type": {
        "text": "CustomEvent<string>"
      },
      "name": "bl-input"
    },
    {
      "type": {
        "text": "CustomEvent<string>"
      },
      "name": "bl-change"
    },
    {
      "type": {
        "text": "CustomEvent<EventDispatcher<ValidityState>>"
      },
      "name": "bl-invalid"
    }
  ],
 ....

We can simply put type.text to the next of event names in baklava-react.ts like described in lit-labs/react documentation. Like:

export const BlTextarea = React.lazy(() =>
  customElements.whenDefined("bl-textarea").then((elem) => ({
    default: createComponent<BlTextareaType>({
      react: React,
      tagName: "bl-textarea",
      elementClass: elem,
      events: {
        onInput: "bl-input" as EventName<CustomEvent<string>>,
        onChange: "bl-change" as EventName<CustomEvent<string>>,
        onInvalid: "bl-invalid" as EventName<CustomEvent<ValidityState>>,
      },
    }),
  }))
);

This can already improve the experience a lot, but currently, I used more advanced event types in my PR for select component and this approach will not be enough for setting types for that. Because Select will accept custom types for its values and its event will provide the selected item with its custom type. So we'll need to use Generic types together with React.lazy. Adding generic types to our Baklava React wrappers is the challenge here and I couldn't find a quick solution for now.

Footnotes

1. You may notice another issue here that bl-invalid event has EventDispatcher type as well, which is wrong. This is another problem that we need to fix but I believe it'll not be that much challenging. ↩

[muratcorlu]

I included this (in a better way, IMO) in my PR about event names: #449 @AykutSarac Please have a look if it works for you.

[AykutSarac]

Works very well, thank you!

[github-actions]

🎉 This issue has been resolved in version 2.0.0-beta.77 🎉

The release is available on:

• npm package (@beta dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This issue has been resolved in version 2.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀