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

update types, deprecate lowercase event handlers #386

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

Conversation

titoBouzout
Copy link
Contributor

@titoBouzout titoBouzout commented Dec 3, 2024

It has been noted that supporting both onclick and onClick makes components a bit more complicated to type and support, as a consumer could use any of both. Literally copying and pasting the issue

oh it's just the same old issue: If JSX.ButtonHTMLAttributes has both onclick and onClick, then when writing a component to wrap it, you either have to pick one or the other Omit<JSX.ButtonHTMLAttributes, "onClick"> or you have to support both. Consider

function PartyButton(props: JSX.ButtonHTMLAttributes<HTMLButtonElement>) {
  let button: HTMLButtonElement | undefined;
  return (
    <button
      {...props}
      ref={fwd(props, ref => (button = ref))}
      onclick={e => (
        triggerConfettiAnimation(button),
        typeof props.onclick === "function" ? props.onclick?.(e) : undefined
      )}
    />
  );
}

now when someone uses this <PartyButton onClick={() => console.log("clicked")}>Example</PartyButton> the onClick doesn't get called

From https://discord.com/channels/722131463138705510/817960620736380928/1313201415182745630 by @webstrand


Kobalte and corvu both use camelCase events.

The eslint plugin also prefers onClick over onclick, and by default will autocorrect accordingly. (But there's an option to ignoreCase) [from @edemaine ]

@titoBouzout titoBouzout changed the title deprecate lowercase event handlers update types, deprecate lowercase event handlers Dec 3, 2024
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