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

improve the JSX type helpers, add a new dispatchEventWithCall(event) method #35

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

Conversation

trusktr
Copy link
Member

@trusktr trusktr commented Oct 13, 2024

feat: improve the JSX type helpers:

  • on* event properties can now be specified to define event props for JSX. The README has an example, plus the jsx-types-*.test.tsx files show all the type test cases.
    • They will be mapped without string values, accepting only functions.
  • For Solid specifically, all the on* properties are also mapped to on:* JSX props with the same types.
  • Any boolean JSX props will also accept "true" and "false" string values.
  • Any number JSX props will also accept number strings, f.e. "1.23".
  • For Solid specifically, automatically map prop types for attr:, prop:, and bool: prefixed JSX props.
    • attr: props will accept only strings.
      • attr: props mapped from boolean JS properties will specifically accept "true" and "false" strings.
      • attr: props mapped from number JS properties will specifically accept number strings, f.e. "1.23".
    • Only boolean JS properties will map to bool: JSX props, and f.e. bool: will not be available for any number props.
      • bool: props will accept only booleans, and not the "true" or "false" strings (otherwise Solid will set the attribute to always exist regardless of the value, and this is not an issue with React props because React props always map to JS properties never attributes when it comes to Lume Elements).
      • The attr: props will accept only "true" or "false" strings.
      • The non-prefixed JSX props, and prop: props, will accept both booleans and "true" or "false" strings.
    • Number JS properties are mapped to JSX props that accept numbers as well as number strings, but similar to boolean properties:
      • The attr: props accept only number strings.
      • And there are no bool: props mapped for number properties.
    • POSSIBLY BREAKING:: This update adds JSX types for event properties, which has a tiny chance of being breaking. If you had a typed JSX prop like onnotevent for an element whose JSX types you defined with ElementAttributes, there might be a type error, but this scenario is unlikely. To migrate, use the prop: prefix to set the JS property instead, for example prop:onnotevent={someValue}. The additional prefixed props could also cause an @ts-expect-error somewhere to start erroring, or a type conflict, in which case some care is needed.

feat: add a new dispatchEventWithCall(event) method

This is similar to dispatchEvent(), but useful for dispatching a non-builtin event and causing any on* method for that event to also be called if it exists.

With builtin events, for example, when the builtin click event is dispatched, the element's .onclick() method is called automatically if it exists. Now we can achieve the same behavior with custom events, so that for example dispatchEventWithCall(new Event('myevent')) will also cause .onmyevent() to be called if it exists.

Note, avoid calling this method with an event that is not a custom event, or you'll trigger the respective builtin on* method twice.

@trusktr
Copy link
Member Author

trusktr commented Oct 13, 2024

@titoBouzout @bigmistqke releasing this tomorrow (Sunday). In particular, check out the type tests to see how it works exactly:

  • src/jsx-types-solid.test.tsx
  • src/jsx-types-react.test.tsx

The on* properties on the class are so that on* JSX props (and on: in Solid JSX) will be mapped.

But in order for it to make sense, like not be just dummy properties, I added the dispatchEventWithCall method to fill that gap. So, if an event method exists, it will be called if dispatchEventWithCall is used. Nice thing about this is that onfoo in JSX now semantically matches with the class definition too. And! Even using prop:onfoo will work.

@trusktr trusktr force-pushed the update-jsx-prop-type-helpers branch from ac9ac7d to e33b46b Compare October 13, 2024 10:07
- `on*` event properties can now be specified to define event props for JSX. The
  README has an example, plus the `jsx-types-*.test.tsx` files show all the type
  test cases.
  - They will be mapped without string values, accepting only functions.
- For Solid specifically, all the `on*` properties are also mapped to `on:*` JSX
  props with the same types.
- Any boolean JSX props will also accept "true" and "false" string values.
- Any number JSX props will also accept number strings, f.e. "1.23".
- For Solid specifically, automatically map prop types for `attr:`, `prop:`, and
  `bool:` prefixed JSX props.
  - `attr:` props will accept only strings.
    - `attr:` props mapped from boolean JS properties will specifically accept
      "true" and "false" strings.
    - `attr:` props mapped from number JS properties will specifically accept
      number strings, f.e. "1.23".
  - Only boolean JS properties will map to `bool:` JSX props, and f.e. `bool:`
    will not be available for any number props.
    - `bool:` props will accept only booleans, and not the "true" or "false"
      strings (otherwise Solid will set the attribute to always exist regardless
      of the value, and this is not an issue with React props because React props
      always map to JS properties never attributes when it comes to Lume
      Elements).
    - The `attr:` props will accept only "true" or "false" strings.
    - The non-prefixed JSX props, and `prop:` props, will accept both booleans
      and "true" or "false" strings.
  - Number JS properties are mapped to JSX props that accept numbers as well as
    number strings, but similar to boolean properties:
    - The `attr:` props accept only number strings.
    - And there are no `bool:` props mapped for number properties.
  - **POSSIBLY BREAKING:**: This update adds JSX types for event properties, which
    has a tiny chance of being breaking. If you had a typed JSX prop like
    `onnotevent` for an element whose JSX types you defined with
    `ElementAttributes`, there might be a type error, but this scenario is unlikely.
    To migrate, use the `prop:` prefix to set the JS property instead, for example
    `prop:onnotevent={someValue}`. The additional prefixed props could also cause an
    `@ts-expect-error` somewhere to start erroring, or a type conflict, in which
    case some care is needed.

**feat:** add a new `dispatchEventWithCall(event)` method

This is similar to `dispatchEvent()`, but useful for dispatching a non-builtin
event and causing any `on*` method for that event to also be called if it
exists.

With builtin events, for example, when the builtin `click` event is dispatched,
the element's `.onclick()` method is called automatically if it exists. Now we
can achieve the same behavior with custom events, so that for example
`dispatchEventWithCall(new Event('myevent'))` will also cause `.onmyevent()`
to be called if it exists.

Note, avoid calling this method with an event that is not a custom event, or
you'll trigger the respective builtin `on*` method twice.
@trusktr trusktr force-pushed the update-jsx-prop-type-helpers branch from e33b46b to 37b7269 Compare October 13, 2024 10:09
@trusktr
Copy link
Member Author

trusktr commented Oct 13, 2024

Hmm, if someone doesn't want the on* methods, we can perhaps provide an additional helper for the JSX to convert a map {eventname: EventType} to the on JSX props. But I think this is good enough for now.

I'm planning to expand dispatchEventWithCall so that it also works with bubbling, like the builtin events. I will delete dispatchEventWithCall, there's a better way to do it with an @event decorator which won't require a new method, and will also support bubbling (the property will simply get/set the function value to add/removeEventListener).

Might also provide an additional helper for non-attribute JS-only props. But that is a bad practice. Nah, not gonna do it. Someone can easily map those to JSX types if they really wish to do so. Not recommended to have an element that works one way in JSX but not compatible with HTML; an antipattern.

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.

1 participant