-
Notifications
You must be signed in to change notification settings - Fork 94
Add type definitions for TypeScript (fixes #10) #12
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #12 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 1 1
Lines 95 95
Branches 37 37
=====================================
Hits 95 95 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just few things.
-
Regarding recent React changes and addition of hooks, it is recommended to use
React.FunctionComponent<>
instead ofReact.SFC<>
. SFC type is going to be deprecated. -
To pass arguments to
<Async>
component, I can't use the way it is done in plain JS:
<Async promiseFn={this.getSession} sessionId={123}/>
Is this intended way how to pass arguments when using TS?:
<Async promiseFn={this.getSession} {...{sessionId: 123}}/>
-
It would be cool to have typed
data
object in callbacks via generics. I tried it when using my own typings and I was not able to make everything work as I would like. I will keep trying, but if anyone could come with a solution, it would be perfect.
If you would like to have a look, here is my current typings file. On my current setup (TS 3.1.4), generics seem to work for base
|
Looks good 👍 I'll update my PR. interface AsyncState<T> {
initialValue?: T
data?: T
error?: Error
isLoading: boolean
startedAt?: Date
finishedAt?: Date
cancel: () => void
run: (...args) => Promise<T>
reload: () => void
setData: (data: T, callback?: () => void) => T
setError: (error: Error, callback?: () => void) => Error
} |
Yes, your |
I've pushed the latest changes. VS Code was complaining about the Also VS Code was complaining about the generic type T not being accessible to static members of Async. Maybe it works regardless? Could you check if this works? |
I placed the index.d.ts file into |
Alright, I'm merging it then. We can always make tweaks later. |
Published in v3.8.2 |
Hey @tomshane, I've been looking at this again and I'm not happy with the way the static members are defined. Their type signature is obviously broken because a static member cannot access the parent class' generic type. I've been fiddling around with it all day but still have not found a solution. So I had to ask: https://stackoverflow.com/questions/53564005/correct-type-definition-for-a-react-class-with-static-member-components Maybe you can take a look at it? |
Yea, I was trying before, but I gave up, because I couldn't make it work. That's why I left the helper components non-generic in my draft. I have found this: microsoft/TypeScript#24018, so it seems it is not supported in the current TS. |
Well they don't have to reference the class generic, they can have their own. I just haven't found a way to do that yet. |
I don't know if that's what you have in mind, but I've tried this:
This way I was able to use helper components without an issue. However, the generic type argument won't be inferred, so the usage must be either:
or:
|
@all-contributors please add @tomshane for review |
I've put up a pull request to add @tomshane! 🎉 |
@all-contributors please add @ghengeveld for code, review, question, blog, doc, design, example, financial, infra, maintenance |
I've put up a pull request to add @ghengeveld! 🎉 |
No description provided.