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

[fredrikhr]

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> → 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.

[JaKXz]

I have a few minor notes, but overall I think this is great and what I was wanting but had absolutely no idea how to do in TS. I like that this feels intuitive to use and would provide a lot of good information to devs.

I need to do more focused reading, but my blunt question is which of the two alternatives is best? From skimming them on my phone, I was drawn to this one to review it a bit more fully but unfortunately I just haven't had a lot of spare time to make a fair judgement.

[JaKXz]

I'm curious what the benefit of creating Type and using the extends keyword is, as opposed to just saying it must be a string. I'm not up to speed on the latest TS trends 😅

[fredrikhr]

In TypeScript you can restrict a type to a constant value.

For a Counter application example this makes it possible to change the following code:

import { FSA } from 'flux-standard-action';

const INCREMENT_COUNTER = 'INCREMENT_COUNTER';
type INCREMENT_COUNTER = typeof INCREMENT_COUNTER;
interface IncrementCounterAction extends FSA {
  type: INCREMENT_COUNTER;
}

to this:

import { FSA } from 'flux-standard-action';

const INCREMENT_COUNTER = 'INCREMENT_COUNTER';
type IncrementCounterAction = FSA<undefined, undefined, typeof INCREMENT_COUNTER>;

removing the need to re-declare the type property. An object using a string, but different to 'INCREMENT_COUNTER' would thus not implement the IncrementCounterAction type.

[hally9k]

@couven92 Why would we want to put the Type generic to be last? This would mean we would always have to explicitly type meta whether the action has meta or not.
I think I would prefer to put tType first as it is the primary property of an FSA and I want to always strongly type my action types for discriminitive unions in redux reducers. Where as quite a lot of my action don't utilise the Meta property.

[hally9k]

My opinionated take on this is on the master branch of this fork if you want to have a look https://github.com/hally9k/flux-standard-action

[fredrikhr]

see #114 (comment)

[fredrikhr]

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 Add TypeScript definitions to enforce that payload and/or meta properties are defined. #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.

[fredrikhr]

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?

[Neaox]

Any movement on this?

[fredrikhr]

@Neaox 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

[hally9k]

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 😄

[hally9k]

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

[fredrikhr]

@hally9k 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.

[JaKXz]

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.

[fredrikhr]

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

[fredrikhr]

@JaKXz 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 → 3.0.0 or 2.0.4 → 2.1.0)

[JaKXz]

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

[JaKXz]

+ flux-standard-action@2.1.0 has been published. Thanks for your patience @couven92 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.

[hally9k]

@couven92 Nice!! 🚀 🚀 🚀

[thisissami]

@hally9k & @couven92 - 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.

[hally9k]

@thisissami 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<"MY-ACTION", MyPayload>

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.

[thisissami]

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 @hally9k?