weixin_40008920
weixin_40008920
2021-01-11 21:08

Add TypeScript definitions to enforce or infer whether properties are required.

Alternative to PR #112 (documented in #109). Introduces breaking changes

  • Introduce FSAWithPayload and ErrorFSAWithPayload
  • Introduce FSAWithMeta and ErrorFSAWithMeta
  • Introduce FSAWithPayloadAndMeta and ErrorFSAWithPayloadAndMeta

  • Introduce FSAAuto and ErrorFSAAuto which infers which FSA type to use based on the type argument being undefined or not.

So FSAAuto (i.e. FSAAuto<undefined, undefined>) maps to FSA, while FSAAuto<any> maps to FSAWithPayload since any does not extend undefined.

I'd be very happy for alternative naming suggestions. Maybe FSAInfer or 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> &rightarrow; FSAAuto<Type, Payload, Meta>. All type arguments are optional, with Type defaulting to string, CustomError defaulting to Error, and Payload and 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.

该提问来源于开源项目:redux-utilities/flux-standard-action

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

16条回答

  • weixin_39543478 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 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 ?

    点赞 评论 复制链接分享
  • weixin_40008920 weixin_40008920 4月前

    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 payload and meta properties (in the FSAWithPayload and FSAWithMeta types) gives us the oppurtunity to add specific documentation comments on these properties, explictly stating that they are required.
    • Inferred requirements. The FSAAuto type automatically detects the usage of undefined arguments for Payload or 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 undefined for either and both Payload and Meta have to be checked. It will be difficult to extend the FSA with 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 payload property to be specified as shown in the test added in bdf8205.

    点赞 评论 复制链接分享
  • weixin_40008920 weixin_40008920 4月前

    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 FSAAuto as a name. Is FSAInfer better? Any other suggestions?

    点赞 评论 复制链接分享
  • weixin_39937312 weixin_39937312 4月前

    Any movement on this?

    点赞 评论 复制链接分享
  • weixin_40008920 weixin_40008920 4月前

    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

    点赞 评论 复制链接分享
  • weixin_39835607 weixin_39835607 4月前

    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 type how 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 😄

    点赞 评论 复制链接分享
  • weixin_39835607 weixin_39835607 4月前

    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

    点赞 评论 复制链接分享
  • weixin_40008920 weixin_40008920 4月前

    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.

    点赞 评论 复制链接分享
  • weixin_39684235 weixin_39684235 4月前

    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.

    点赞 评论 复制链接分享
  • weixin_40008920 weixin_40008920 4月前

    ok, that's great. Pushed new changes and updated the PR description on top :)

    点赞 评论 复制链接分享
  • weixin_40008920 weixin_40008920 4月前

    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.0.4 &rightarrow; 3.0.0 or 2.0.4 &rightarrow; 2.1.0)

    点赞 评论 复制链接分享
  • weixin_39684235 weixin_39684235 4月前

    Don't worry about the package version, I will follow the release process, later tonight hopefully!

    点赞 评论 复制链接分享
  • weixin_39684235 weixin_39684235 4月前

    + flux-standard-action.1.0 has 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.

    点赞 评论 复制链接分享
  • weixin_39835607 weixin_39835607 4月前

    Nice!! 🚀 🚀 🚀

    点赞 评论 复制链接分享
  • weixin_39543478 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 Type will be just string. In our codebase, we've had to update 90% of our FSAs to have a string tacked on at the beginning. Seems like the thing with a default value that will be used often shouldn't be first.

    点赞 评论 复制链接分享
  • weixin_39835607 weixin_39835607 4月前

    Hey, one of the most important requirements to be able to add type safety to redux reducers is having the action parameter in the reducer function typed as a union of action types. Specifically a discriminated union that discriminates on the type property. 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 type property with a string literal e.g. "MY-ACTION" as opposed to the looser string type. e.g:

    
       FSA
    

    For this reason, in my code I always type the type property with a string literal and that was the reasoning behind making type the first argument in the generic signature. Does this make sense? Let me know if I missed the point.

    点赞 评论 复制链接分享

相关推荐