Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(typings): synthetic event #740

Merged
merged 3 commits into from
Jan 17, 2019
Merged

fix(typings): synthetic event #740

merged 3 commits into from
Jan 17, 2019

Conversation

kuzhelov
Copy link
Contributor

@kuzhelov kuzhelov commented Jan 17, 2019

This PR explicitly specifies generic param (as HTMLElement) of type that is used for synthetic event arg in component's event handlers.

This is necessary for client libraries to avoid explicit cast from Element (currently provided by Stardust) to HTMLElement which is expected by client's code.

TODO

  • add changelog entry

@@ -31,7 +31,10 @@ export type ReactChildren = React.ReactNodeArray | React.ReactNode
export type ReactPropsStrict<T> = { [K in keyof T]: NullableIfUndefined<T[K]> }
export type ReactProps<T> = Extendable<ReactPropsStrict<T>>

export type ComponentEventHandler<TProps> = (event: React.SyntheticEvent, data: TProps) => void
export type ComponentEventHandler<TProps> = (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export type ComponentEventHandler<TProps> = (
export type ComponentEventHandler<TProps, TElem = HTMLElement> = (

Can we make this a generic?

Copy link
Contributor Author

@kuzhelov kuzhelov Jan 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, although not sure whether it is necessary to introduce this variability now, as it is pretty much clear that it will always be about HTMLElement for this handler's type - thus, not sure if we should make this type to be customizable (as it will, probably, will be an indication of some improper assumptions)

@kuzhelov kuzhelov merged commit 11614e0 into master Jan 17, 2019
@kuzhelov kuzhelov deleted the fix/synthetic-event-type branch January 17, 2019 12:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🧰 fix Introduces fix for broken behavior. 🚀 ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants