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

Add TypeScript typings corresponding to redux@next #812

Closed
wants to merge 5 commits into from

Conversation

pelotom
Copy link

@pelotom pelotom commented Oct 24, 2017

Add built-in TypeScript typings copied from DefinitelyTyped but modified to accommodate upcoming revamped redux typings on the next branch (see redux #2563).

index.d.ts Outdated
/**
* The single Redux store in your application.
*/
store?: Store<any, any>;
Copy link
Author

Choose a reason for hiding this comment

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

This is the main change from what's on DefinitelyTyped, which requires the new redux typings. Without it, users will probably run into problems instantiating Provider elements in the presence of TypeScript 2.6's --strictFunctionTypes.

@pelotom
Copy link
Author

pelotom commented Oct 24, 2017

This is not ready to merge until it can depend on the yet-to-be-released Redux 4 which will have the new typings.

@timdorr
Copy link
Member

timdorr commented Oct 24, 2017

Thanks, @pelotom! Can any TS experts weigh in on this?

index.d.ts Outdated
type Omit<T, K extends keyof T> = Pick<T, Diff<keyof T, K>>;

export interface DispatchProp<S> {
dispatch?: Dispatch<S>;
Copy link

@aikoven aikoven Oct 24, 2017

Choose a reason for hiding this comment

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

Isn't the Dispatch type parameter an action type now?

@aikoven
Copy link

aikoven commented Oct 24, 2017

Looks good to me.

Tests are missing though.

@timdorr
Copy link
Member

timdorr commented Oct 24, 2017

Ah yes. @pelotom Can you pull in typescript-definition-tester to get these tested? Might not work until Redux's next branch gets merged.

index.d.ts Outdated
export interface DispatchProp<S> {
dispatch?: Dispatch<S>;
export interface DispatchProp {
dispatch?: Dispatch<{}>;
Copy link
Author

Choose a reason for hiding this comment

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

No need to have a type parameter at all here since we were only instantiating it with any. Hard-coding it to {} instead of any because it's in a contravariant position.

@pelotom
Copy link
Author

pelotom commented Oct 24, 2017

Actually it looks like these definitions are compatible with both the current and next redux typings.

@pelotom
Copy link
Author

pelotom commented Oct 25, 2017

Hm, I just saw #541, and I'm not sure about the relative merits of those typings vs these. The test cases are undoubtedly more comprehensive there. Is there any reason it seems to have been abandoned? Just no one got around to resolving merge conflicts?

@pelotom
Copy link
Author

pelotom commented Oct 25, 2017

Closing this for now as I think #541 is a better foundation to build the next typings off of.

@pelotom pelotom closed this Oct 25, 2017
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

Successfully merging this pull request may close these issues.

3 participants