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

Typed the props argument on the selectors created by createSelector, and added typings test cases. #115

Merged
merged 8 commits into from
Jun 21, 2016

Conversation

geon
Copy link
Contributor

@geon geon commented Apr 19, 2016

The test is done by simply running tsc in the /tests/typings/ folder. If it compiles, everything is fine.

@geon
Copy link
Contributor Author

geon commented Apr 19, 2016

The tests should probably be runnable by Travis CI. Anyone know how to fix?

@codecov-io
Copy link

codecov-io commented Apr 19, 2016

Current coverage is 100%

No coverage report found for master at 7815c3c.

Powered by Codecov. Last updated by 7815c3c...8a9b4e8

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 87df18a on geon:master into 7815c3c on reactjs:master.

@@ -5,22 +5,22 @@

declare module Reselect {

type Selector<TInput, TOutput> = (state: TInput, props?: any) => TOutput;
type Selector<TInput, TProps, TOutput> = (state: TInput, TProps, props?: TProps) => TOutput;
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this a typo? That looks like you accidentally included a required implicit any parameter named TProps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. Yes.

@coveralls
Copy link

coveralls commented May 4, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling d5b46b8 on geon:master into 7815c3c on reactjs:master.

@@ -0,0 +1,22 @@

export type RootState = {
items: {[key: string]: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you convert tabs to spaces to match the other files? It's a small thing, but it'll help avoid whitespace changes on future diffs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@coveralls
Copy link

coveralls commented Jun 20, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 8a9b4e8 on geon:master into 7815c3c on reactjs:master.

@threehams threehams merged commit ee8f024 into reduxjs:master Jun 21, 2016
@ellbee
Copy link
Collaborator

ellbee commented Jun 21, 2016

🎉 Thanks for this @geon!

And thanks @threehams for reviewing and merging. Much appreciated!

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.

None yet

6 participants