Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

types: EventHandler returning (seemingly) incorrect type #4581

Closed
1 task done
rschristian opened this issue Nov 29, 2024 · 2 comments · Fixed by #4584
Closed
1 task done

types: EventHandler returning (seemingly) incorrect type #4581

rschristian opened this issue Nov 29, 2024 · 2 comments · Fixed by #4584
Labels

Comments

@rschristian
Copy link
Member

  • Check if updating to the latest Preact version resolves the issue

Describe the bug
Found this trying to help Hypothesis upgrade their frontend-shared repo.

#4538 seems to be the cause of a regression in the following type, resulting in the following error:

Property 'call' does not exist on type 'GenericEventHandler<HTMLInputElement>'.
  Property 'call' does not exist on type 'EventHandlerObject<TargetedEvent<HTMLInputElement, Event>>'.

Not really sure what that type change is even doing, need more time to investigate but wanted to get this opened.

To Reproduce

TS Playground

import { JSX } from "preact";

function Checkbox({ onChange }: JSX.HTMLAttributes<HTMLInputElement>) {

  function handleChange(this: void, event: JSX.TargetedEvent<HTMLInputElement>) {
    onChange?.call(this, event);
           // ^ Error here
  }

  return <input onChange={handleChange} />
}

Expected behavior

Shouldn't be any error

@JoviDeCroock
Copy link
Member

Hmm, the whole bivariancehack situation is concerning already. I wonder whether we can get away with a similar fix to #4547

@lilnasy
Copy link
Contributor

lilnasy commented Nov 29, 2024

I think the error really is expected. Starting with the latest version, Checkbox can be passed something other than a function.

For example, with this usage:

class Form extends Component {
    handleEvent(event: Event) {
        ....
    }
    
    render() {
        return <Checkbox onChange={this}/>
    }
}

The userland fix would be something like this:

import type { JSX } from "preact";

function Checkbox({ onChange }: JSX.HTMLAttributes<HTMLInputElement>) {
    function handleChange(this: void, event: JSX.TargetedEvent<HTMLInputElement>) {
        // onChange?.call(this, event);
        onChange && "handleEvent" in onChange
            ? onChange.handleEvent(event)
            : onChange?.call(this, event)
    }
    return <input onChange={handleChange} />
}

Although, I expect this to be a widely used pattern, so maybe there should be something to make migration easier. That could be a helper type so that Checkbox can say, "I am actually different from an input element because I accept a narrower type for event handlers":

import type { JSX } from "preact";

type FunctionOnly<Element extends EventTarget> = {
    [Key in keyof JSX.HTMLAttributes<Element>]:
        Exclude<JSX.HTMLAttributes<Element>[Key], JSX.EventHandlerObject<JSX.TargetedEvent>>
}

function Checkbox({ onChange }: FunctionOnly<HTMLInputElement>) {
    function handleChange(this: void, event: JSX.TargetedEvent<HTMLInputElement>) {
        onChange?.call(this, event);
    }
    return <input onChange={handleChange} />
}

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants