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

2.1.0 breaking change; async start() with payload is now forbidden. #24

Closed
estaub opened this issue May 21, 2017 · 6 comments
Closed

2.1.0 breaking change; async start() with payload is now forbidden. #24

estaub opened this issue May 21, 2017 · 6 comments

Comments

@estaub
Copy link

estaub commented May 21, 2017

So... I was using a real payload in async .start(), and now it won't even compile, which I suppose is a good thing, since it doesn't work. Clearly this is intentional; there's even a test to make sure payloads aren't allowed.

Semver, please? If you're going to deliberately break stuff that works now, bump the major rev number.

Falling back to 2.0 for now. Please let me know if this (i.e., forbidding .start() payload) is permanent.

@aikoven
Copy link
Owner

aikoven commented May 22, 2017

I am truly sorry about this, payloads are meant to be allowed for .started() action creators, the 2.1.0 version only makes it possible to skip the payload if its type is undefined. As you can see here we only added a new signature and old ones are still there. But for some reason TypeScript selects the wrong signature when you declare async action creator with non-undefined payload type.

I will revert and deprecate version 2.1. You can use 2.0 for now since there weren't any other changes.

@aikoven
Copy link
Owner

aikoven commented May 22, 2017

Released 2.2.0.

@estaub
Copy link
Author

estaub commented May 22, 2017

@aikoven Thanks, and sorry to go off on you a bit. I misread one the tests, then inferred nonexistent intent.

@tblaisot
Copy link

tblaisot commented May 27, 2017

@aikoven Couldn't you have the same result by declaring the payload field of Action non mandatory ? I tried this solution (with all fields related to payload like params, etc...) and i think you lose nothing in term of the security of the typings.
By doing so you can even remove the EmptyActionCreator type.

@aikoven
Copy link
Owner

aikoven commented May 29, 2017

@tblaisot We'd lose the ability for the compiler to tell us if there is a payload or not, which we currently have with strictNullChecks enabled.

@aikoven
Copy link
Owner

aikoven commented Jun 21, 2017

I'm going to close this now, please feel free to reopen if there's a case.

@aikoven aikoven closed this as completed Jun 21, 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

No branches or pull requests

3 participants