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

TypeScript JSX focus events don't have consistent case #4289

Closed
CodingDoug opened this issue Feb 21, 2024 · 5 comments · Fixed by #4307
Closed

TypeScript JSX focus events don't have consistent case #4289

CodingDoug opened this issue Feb 21, 2024 · 5 comments · Fixed by #4307

Comments

@CodingDoug
Copy link

The focus events don't have consistent casing in their names with respect to other event attribtes:

preact/src/jsx.d.ts

Lines 1563 to 1571 in a003d42

// Focus Events
onFocus?: FocusEventHandler<Target> | undefined;
onFocusCapture?: FocusEventHandler<Target> | undefined;
onfocusin?: FocusEventHandler<Target> | undefined;
onfocusinCapture?: FocusEventHandler<Target> | undefined;
onfocusout?: FocusEventHandler<Target> | undefined;
onfocusoutCapture?: FocusEventHandler<Target> | undefined;
onBlur?: FocusEventHandler<Target> | undefined;
onBlurCapture?: FocusEventHandler<Target> | undefined;

The four in question should probably be onFocusIn, onFocusInCapture, onFocusOut, and onFocusOutCapture.

@marvinhagemeister
Copy link
Member

FYI: Preact supports all lowercase event name to align with the web. I think the Capture suffix is because of a check in our source for capture events that's case sensitive. Might make sense to loosen that restriction. Off the top of my head I'm not sure if we support camelcasing them all natively or only in compat. Something worth to check.

@CodingDoug
Copy link
Author

@marvinhagemeister VSCode gives me an uncomfortable error if the JSX attribute doesn't match exactly. For example:

Type '{ children: Element[]; id: string; class: string; onFocusin: undefined; }' is not assignable to type 'HTMLAttributes'.
Property 'onFocusin' does not exist on type 'HTMLAttributes'. Did you mean 'onfocusin'? ts(2322)

@marvinhagemeister
Copy link
Member

Yeah onFocusin is something we'll never support. In your original message you wrote onFocusIn (note the uppercase letter I). Where is this new form of casing coming from?

@CodingDoug
Copy link
Author

In my original message, I suggested that the definitions for these event attributes are incorrectly cased compared to the rest of the event attributes (scan the entire list in the file I cited - every single one of them is camel cased except for four of the focus events). I suggested alternatives that would bring them into consistency.

I think I may have been unclear after that - I'm not saying that I want to use onFocusin. I want to use camel cased attributes as they are defined in jsx.d.ts. Are you saying that JSX should be using all lowercase instead, and the definitions in jsx.d.ts don't really matter?

@marvinhagemeister
Copy link
Member

Are you saying that JSX should be using all lowercase instead, and the definitions in jsx.d.ts don't really matter?

No, I was just giving historical context. We should support lowercased events like they are specced in web standards + camelCase. Might just be a matter of updating the type definitions in our JSX types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants