Make meta optional

[wub]

(Edited) Don't use this PR; it'll cause issues, and better things on the way from the TS team.

[JaKXz]

Thanks for the patch! For some reason, I'm not seeing good errors on travis, but I tried the test command locally and it printed:

test/typings-test.ts(50,21): error TS2532: Object is possibly 'undefined'.
test/typings-test.ts(53,9): error TS2322: Type 'string | undefined' is not assignable to type 'string'.
  Type 'undefined' is not assignable to type 'string'.
test/typings-test.ts(71,21): error TS2532: Object is possibly 'undefined'.
test/typings-test.ts(74,9): error TS2322: Type 'string | undefined' is not assignable to type 'string'.
  Type 'undefined' is not assignable to type 'string'.

cc @unional

[wub]

Oh whoops, that's laziness on my part, I'll fix that!

[coveralls]

Coverage remained the same at 100.0% when pulling 58840bc on wub:master into 5eb76c5 on acdlite:master.

[unional]

I'm not sure if this is the right change.

If you define a meta, do you expect that it can be optional?

If it is optional, then may be one option is to specify it as YourMeta | undefined

[wub]

Good point! I find that I don't really use meta at all.

If I could do x: FSA<foo> instead of x: FSA<foo, any | undefined> everywhere it would look a bit tidier, but I don't know how complex that would make the typings.

Also, I could just make my own type X = FSA<foo, any | undefined>, couldn't I.

[unional]

That will be possible when generic default lands.

In the mean time, I don't think overloading would work, but can give it a try

[wub]

Great, thanks for the feedback @unional. Up to you guys!

[unional]

You are welcome.

It is a touchy point in TypeScript.
In the typings in DT, it uses FSA<Payload> & Meta<Meta> to get around this problem.
However, IMO it does not do it justice as it is really a hack because when you define a FSA with meta, you are thinking of it as one type instead of an interaction of types. But it is IMO and discussion is always welcome.

The current typing is more strict and the good thing about it is that when generic default and undefined as optional is accepted and landed, there won't be any breaking change to the consumer of this library.
We can just loosen up the typings.

Here are references to the issues if you are interested:
microsoft/TypeScript#2175
microsoft/TypeScript#12400

[JaKXz]

@wub @unional thanks for the discussion, it's been informative to me too. Unless I'm misunderstanding something it seems like this is blocked by those Typescript issues? Specifically though, this point makes the most sense to me:

If you define a meta, do you expect that it can be optional?

I would motion to close this PR in the meanwhile, possibly in favour of another one that "loosens" up the typings... though I'm not sure about what that looks like. TBPH I'm pretty happy with the way things are for now, but if others feel strongly that this needs to change somehow then I'm open to changes. :)

[unional]

FYI generic default is available in ts@next. XD

[wub]

@JaKXz Cheers! See you on the other side of ts@next ;)

[ghost]

In the JSDocs, and README, it's still listed as optional (so is Payload).

 /**
   * The optional `meta` property MAY be any type of value.
   * It is intended for any extra information that is not part of the payload.
   */
  meta: Meta;

Shouldn't it be the same as error? (meta?: Meta;)

[zowers]

will this be resolved?

[unional]

will this be resolved?

Do you mean fixing the readme?

[zowers]

I mean when the PR will be actually merged or there will be done anything to mark meta as optional

[unional]

As discusses above, keeping meta as require field provides a more accurate type when you use meta.
Currently, there is a limitation on TypeScript side that you need to explicitly define undefined in the generic, but hopefully when it is fixed on TS land, this limitation can be removed and you can just use it as FSA<YourPayload> instead of FSA<YourPayload, undefined>

[zowers]

Oh, cool! Thanks for the explanation, it was not clear from the above comments that FSA<YourPayload, undefined> is compatible with { type, payload } (without explicit meta: undefined)

[wub]

Yeah, sorry, @zowers (and anyone else), my PR is incorrect and will cause issues! I'll edit the parent.

[leesiongchan]

I thought we can set something like this?

export interface FluxStandardAction<Payload = undefined, Meta = undefined> {
  ...
}

[wub]

@leesiongchan The first conversation happened before TypeScript 2.3, which introduced defaults for generics (like your example). But yes, we can do that now! I'll submit a request.

[wub]

See #94