Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Contribute an alternative typing example for forms sample#24

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
swyxio merged 3 commits intotypescript-cheatsheets:masterfromTomasHubelbauer:patch-1
Jun 8, 2018
Merged

Contribute an alternative typing example for forms sample#24

swyxio merged 3 commits intotypescript-cheatsheets:masterfromTomasHubelbauer:patch-1
Jun 8, 2018

Conversation

@TomasHubelbauer
Copy link
Contributor

I like to use this, I think it boils down to a preference, but maybe it's worthwhile to showcase both?

@ghost
Copy link

ghost commentedJun 8, 2018
edited by ghost
Loading

Hi,
Why just not typing the param (e) with the typings on onChange handler in React (when needed)?

onMaskClick=(e:React.MouseEvent<HTMLDivElement>)

Also, some events may don't have params so developer should not be injecting the types, but in your case you add unnecessary typings that may affect readability.

What you think@sw-yx ?

@ghost ghost requested a review fromswyxioJune 8, 2018 13:28
@TomasHubelbauer
Copy link
ContributorAuthor

TomasHubelbauer commentedJun 8, 2018
edited
Loading

Like I said, this is an alternative, either you describe the type in the method signature, or you use the provided handler type in React TypeScript types and then you don't have to specify the argument types yourself as you are enforcing a type of the whole delegate.

Essentially these is a shortcut to not having to specify the delegate type yourself, you already have it in React, but it's not really a shortcut, because you have to type about the same amount anyway. But for me I look to use the out of the box stuff wherever possible as a matter of principle, even if in this case it's same different in effort to type.

cgeiershouse reacted with thumbs up emoji

@swyxio
Copy link
Collaborator

swyxio commentedJun 8, 2018
edited
Loading

interesting. I would say this is similar enough that we should just make it a one-liner to let people know it exists. doesn't really need more than that. I would also put in your explanation in the details/summary tag so that people can learn about delegate types (I don't really know about them).

out of curiosity@TomasHubelbauer - how did you learn about React.ChangeEventHandler? I have never even seen it before. Is there some other resources you recommend? thank you for contributing btw.

@TomasHubelbauer
Copy link
ContributorAuthor

I found it on my own reading the TypeScript types for React. I agree with making it a one-liner for comparison.

My preference for this comes from the fact that this is an out of the box type which covers not only the argument types, but the type of the entire handler method, so while it's the same amount of work to type this, you are using a "blessed" type instead of making up an ad-hoc inferred type ((…args…) => void) on the spot. That's what I think is the best justification for going with this as opposed to what is displayed currently.

swyxio reacted with thumbs up emoji

@swyxio
Copy link
Collaborator

swyxio commentedJun 8, 2018
edited
Loading

cool, I'll make minor edits and merge. I would argue the opposite, the less@types/react specific API I have to keep in my head, the better. I don't need to remember the name of React's blessed type, I can just write out the shape of the function signature. this is a muuuuch more common pattern across many other typed functional language paradigms, and even applies to using Typescript outside of React. but I'll include your point of view.

TomasHubelbauer, neilchikani, and rolandsaven reacted with thumbs up emoji

shorten React.ChangeEventHandler example and move to discussion
@swyxioswyxio merged commitea6419f intotypescript-cheatsheets:masterJun 8, 2018
@swyxio
Copy link
Collaborator

thanks for the contribution!!

Guanyuuu reacted with thumbs up emojiTomasHubelbauer reacted with heart emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@swyxioswyxioAwaiting requested review from swyxio

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@TomasHubelbauer@swyxio

[8]ページ先頭

©2009-2025 Movatter.jp