Skip to content

refactor(types/actions): allow type-setting args on ActionCreator #3797

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

Merged
merged 2 commits into from
Jul 14, 2020
Merged

refactor(types/actions): allow type-setting args on ActionCreator #3797

merged 2 commits into from
Jul 14, 2020

Conversation

zanona
Copy link
Contributor

@zanona zanona commented Jun 9, 2020

Following up #3541 (comment)

We are allowing Action creators to be typed by declaring their return type as well as the argument type so function calls keep the expected parameter types.


name: "allow type-setting args on ActionCreator"
about: improving type declarations on Redux

PR Type

Does this PR add a new feature, or fix a bug?

It sort of adds a new feature, but it's opt-in for the type usage defaulting to the same as the previous behaviour.

Why should this PR be included?

Allow developers to use Redux provided typings in order to implement ActionCreators

Checklist

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Is there an existing issue for this PR?
    • link issue here
  • Have the files been linted and formatted?
  • Have the docs been updated to match the changes in the PR?
  • Have the tests been updated to match the changes in the PR?
  • Have you run the tests locally to confirm they pass?

New Features

What new capabilities does this PR add?

What docs changes are needed to explain this?

Bug Fixes

What is the current behavior, and the steps to reproduce the issue?

What is the expected behavior?

How does this PR fix the problem?

@netlify
Copy link

netlify bot commented Jun 9, 2020

Deploy preview for redux-docs ready!

Built with commit a30fe8f

https://deploy-preview-3797--redux-docs.netlify.app

@zanona
Copy link
Contributor Author

zanona commented Jun 9, 2020

I'm not quite sure if there are tests for TS typings. Please let me know if I can add those in, if necessary. Thanks

@timdorr
Copy link
Member

timdorr commented Jun 10, 2020

Because it's a rest parameter, this is enforcing a uniform type for every argument. However, it would be possible to support non-uniform types like so:

interface ActionCreator<A, P extends any[] = any[]> {
  (...args: P): A
}

That would allow you to separately type each argument.

The counter-point would be that you must pass an array type, such as ActionCreator<MyAction, string[]>. But I think that's OK to get more fine-grained control there.

@zanona
Copy link
Contributor Author

zanona commented Jun 10, 2020

The counter-point would be that you must pass an array type, such as ActionCreator<MyAction, string[]>. But I think that's OK to get more fine-grained control there.

👏 That's a very nice idea! I'd guess it could look easier to the eye, having it declared as ActionCreator<MyAction, [string]>?

So we could even have ActionCreator<MyAction, [string, number, string[]]> 😎

Thanks for the review.

@timdorr
Copy link
Member

timdorr commented Jul 14, 2020

OK, I'm cool with this as-is. Thanks!

@timdorr timdorr merged commit 380e8b0 into reduxjs:master Jul 14, 2020
webMasterMrBin pushed a commit to webMasterMrBin/redux that referenced this pull request Aug 21, 2021
webMasterMrBin pushed a commit to webMasterMrBin/redux that referenced this pull request Aug 21, 2021
webMasterMrBin pushed a commit to webMasterMrBin/redux that referenced this pull request Aug 21, 2021
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.

2 participants