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

10.0.0-rc.3 preact/compat typescript incompatibilities #1930

Open
bvella opened this issue Sep 13, 2019 · 5 comments
Open

10.0.0-rc.3 preact/compat typescript incompatibilities #1930

bvella opened this issue Sep 13, 2019 · 5 comments

Comments

@bvella
Copy link

bvella commented Sep 13, 2019

First off, well done on this excellent library. It boggles my mind how smaller this is from react.

I have some existing react libraries and a react app which I am trying out with preact. It is working fine but I had to do some modifications to the typescript typings as below:

I added the following definitions to compat/src/index.d.ts :

export type Component<P = {}, S = {}, SS = any> = preact.Component<P, S>;
export type ReactElement<P = {}, S = any> = preact.VNode<P>;
export import FunctionComponent = preact.FunctionComponent;
export import ComponentClass = preact.ComponentClass;
export import ComponentType = preact.ComponentType;
export import Context = preact.Context;
export import ReactNode = preact.ComponentChildren;
export type CSSProperties = string | {[key: string]: string | number};
export type AllHTMLAttributes<T> = JSX.HTMLAttributes;

I also changed jsx to a global namespace in src/jsx.d.ts:

...
export = JSX;
		
declare global {
export namespace JSX {
...
}
}

I also noticed that there are some subtle types missing, like e.g. the target in the onChange event for an input tag is not typed. I had this jsx line in which the event's target attribute was correctly inferred to be an html input element from the react typings:

<input type="text" name="name" onChange={ e => setName(e.target.value) } />

that I had to change to the following as the preact event handlers all use the same event definition with target being untyped:

<input type="text" name="name" onChange={ e => setName((e.target as HTMLInputElement).value) } />

Are these by design or is there a reason I am not aware of for these missing types? I can go through the react typings and copy over some stuff to improve type checking, if you think this is of benefit to preact. These would be typescript typing updates only, requiring no changes in preact but improving greatly both switching to preact from react and fix subtle type checking errors like the event above.

@marvinhagemeister marvinhagemeister changed the title 10.0.0-rc.3 preact/compat typescript imcompatibilities 10.0.0-rc.3 preact/compat typescript incompatibilities Sep 16, 2019
@marvinhagemeister
Copy link
Member

The namespacing of JSX is done one purpose. It was initially started in #1036 and if you follow the git history, you'll find a few PRs that built on that.

Regarding the incorrect target types: That's definitely a missing feature in our types. We'd welcome any PRs that improve that 👍 💯

@ericlery
Copy link

ericlery commented Dec 8, 2022

There's still no type safety for event handling in Preact till this day?

@kasvith
Copy link

kasvith commented Dec 9, 2022

Wow, jumped into the same issue and realized its a bug on preact

@alextompkins
Copy link

Hey I might be missing something here but the typings for JSX.TargetedEvent<Target extends EventTarget = EventTarget> still seem wrong as of Preact 10.15.1. Specifically that event.target is always of type EventTarget rather than the provided generic Target.

Should I open a PR for this or is somebody already on it?

@tjcrowder
Copy link

@alextompkins - Target controls the type of currentTarget, not target, since target may be a descendant element. The idea is that you can specify the type argument of the element you're attaching the event handler to (which will be currentTarget), since you know what kind of element that will be. This is touched on in #2301.

For instance, this (fairly silly) example works:

const handleClick = (event: JSX.TargetedMouseEvent<HTMLButtonElement>) => {
    event.preventDefault();
    console.log(`Value: ${event.currentTarget.value}`);
};

return (
    <button value={value} onClick={handleClick}>
        <span>{children}</span>
    </button>
);

...whereas with event.target.value instead it would fail because value doesn't exist on target since target has the type EventTarget (and very well may not be the button element; it's probably going to be the span instead).

Side note: See #2706, those event types may be moving out of the JSX namespace.

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

No branches or pull requests

6 participants