Oh interesting. So in that case, when you create a reducer, you set the action paramater's type as a union of the possible very specific
FSAs (based off the
Type), and then when you switch based off
action.type within the reducer - it's effectively a type guard? am I understanding that correctly ?
Add TypeScript definitions to enforce or infer whether properties are required.
Alternative to PR #112 (documented in #109). Introduces breaking changes
ErrorFSAAutowhich infers which FSA type to use based on the type argument being
FSAAuto<undefined, undefined>) maps to
FSAAuto<any> maps to
any does not extend
I'd be very happy for alternative naming suggestions. Maybe
FSAAutoRequire is better?
UPDATE: Because of more sensible usability, the
Type generic argument for FSA-types has been moved to the first position (i.e.
FSA<Payload, Meta, Type> →
FSAAuto<Type, Payload, Meta>. All type arguments are optional, with
Type defaulting to
CustomError defaulting to
Meta defaulting to
undefined allowing for
FSA to automatically become
FSA<string, undefined, undefined>.
Introducing the generic type argument
Type is inspired from PR #113, however the decision was made to introduce a breaking change by changing the order of the generic type arguments.
- weixin_39543478 4月前
Oh interesting. So in that case, when you create a reducer, you set the action paramater's type as a union of the possible very specific点赞 评论 复制链接分享
Okay, first off, from what I can tell both my suggestion and #112 would work. I am going to argue for my suggestion (i.e. #114).
- By redeclaring the
metaproperties (in the
FSAWithMetatypes) gives us the oppurtunity to add specific documentation comments on these properties, explictly stating that they are required.
- Inferred requirements. The
FSAAutotype automatically detects the usage of undefined arguments for
Meta. In cases where you actually specify a type, you probably want the property to be required.
- Obviously a very subjective view, but I'd argue that the conditional alias of the existing types is fairly easy to understand.
Disadvantages: * The condition statement is quite convoluted due to the fact that
undefinedfor either and both
Metahave to be checked. It will be difficult to extend the
FSAwith new additional optional/required properties. The Pick-method of #112 will be better suited to cope with future extensions. * Using
FSAAuto<string | undefined>still requires the
payloadproperty to be specified as shown in the test added in bdf8205.点赞 评论 复制链接分享
- By redeclaring the
And if we go for my option, can we please agree on a name for the automatically inferred requiements type? I do not really like
FSAAutoas a name. Is
FSAInferbetter? Any other suggestions?点赞 评论 复制链接分享
- weixin_39937312 4月前
Any movement on this?点赞 评论 复制链接分享
I'll do the rebase now, and make verything good again, but well... This has been ready for merge since 10th November 2018.
Unless someone objects to my naming, but that did not seem to be the case点赞 评论 复制链接分享
Hey, the master branch is basically unusable in the context of my redux application, this appears to address all the issues I have though. If we can't strong type
typehow are we supposed to use discriminate unions of actions in our reducers? I feel like like I may be missing something. Is this likely to be merged in and released? Is there an npm package of this branch I should use in the mean time? For what it's worth the FSAAuto branch would be my preference. Great work BTW 😄点赞 评论 复制链接分享
After reading into this a bit further I am of the opinion that we should switch the order of the generics around to
<Type, Payload, Meta>Heres a fork showing that approach: https://github.com/hally9k/flux-standard-action/blob/master/src/index.d.ts点赞 评论 复制链接分享
yes, switching the arguments around would also be my preference. However, that would be a breaking change that I did not want to introdruce with this PR. I suggest a new issue or PR with these changes, since that really is not part of the changes proposed here.点赞 评论 复制链接分享
Hi folks, sorry for the delay on this. I think I would prefer merging the most performant version and just releasing that. No need to worry about breaking changes for now.点赞 评论 复制链接分享
ok, that's great. Pushed new changes and updated the PR description on top :)点赞 评论 复制链接分享
because of breaking changes this PR should probably be merged with a new version number? Do you want me to increase the version here in the PR? Major increase? (i.e.
2.1.0)点赞 评论 复制链接分享
Don't worry about the package version, I will follow the release process, later tonight hopefully!点赞 评论 复制链接分享
+ flux-standard-action.1.0has been published. Thanks for your patience and others, please give it a test!
There are some updates I would like to make to the tests and what not but I didn't want them to block the release any more.点赞 评论 复制链接分享
Nice!! 🚀 🚀 🚀点赞 评论 复制链接分享
- weixin_39543478 4月前
& - I'm curious, why did you guys want to change the ordering to
<Type, Payload, Meta>? This seems very counterintuitive to me, considering that very often
Typewill be just
string. In our codebase, we've had to update 90% of our
FSAs to have a
stringtacked on at the beginning. Seems like the thing with a default value that will be used often shouldn't be first.点赞 评论 复制链接分享
Hey, one of the most important requirements to be able to add type safety to redux reducers is having the
actionparameter in the reducer function typed as a union of action types. Specifically a discriminated union that discriminates on the
typeproperty. This is important because then, in the scope of the switch case for a specific action, typescript knows what payload and meta to expect and can therefore provide type safety per individual action. We achieve this by strictly typing the
typeproperty with a string literal e.g.
"MY-ACTION"as opposed to the looser
For this reason, in my code I always type the
typeproperty with a string literal and that was the reasoning behind making
typethe first argument in the generic signature. Does this make sense? Let me know if I missed the point.点赞 评论 复制链接分享