Make meta optional (default to undefined)
#94
Conversation
|
Sorry, not an expert in versioning - what should this be bumped to? |
There was a problem hiding this comment.
This LGTM! don't worry about the versioning :) I'll probably release it as a patch.
Perhaps it would be nice to document the typings in the README? do you think you can take that on as well?
cc @unional in case you have any thoughts.
|
I'm ok with it. One issue remain is this: microsoft/TypeScript#12400 (comment) That's why I didn't add the default generics yet. You can see type declaration ( |
| @@ -26,26 +26,26 @@ export interface FluxStandardAction<Payload, Meta> { | |||
| meta: Meta; | |||
There was a problem hiding this comment.
Isn't this supposed to be optional?
There was a problem hiding this comment.
Nope. That's the whole point. It should not be optional. See #53 and related discussion.
There was a problem hiding this comment.
Hey, sorry to butt in here. Just hit this issue and am a bit confused... Currently I have this issue:
const x: FSA<string, undefined> = { type: "foo", payload: "hello" }Type '{ type: string; payload: string; }' is not assignable to type 'FluxStandardAction<string, undefined>'.
Property 'meta' is missing in type '{ type: string; payload: string; }'.
So setting the type to undefined does not make the attribute optional.
I think the DefinitelyTyped defs handle this in a more appropriate way.
There was a problem hiding this comment.
We discussed that here: #53 (comment)
It is debatable on which is a better solution.
I would say when microsoft/TypeScript#12400 (comment) is resolved, we will be just fine.
There was a problem hiding this comment.
Ah yeah, makes sense. It's just annoying having to add extra code to satisfy types, but a small price to pay I suppose. :)
|
Okay so unless anyone is opposed I'm going to merge this and release it as a patch today :) |
|
In related news, #100 would be a prime candidate for another patch like this one... |
No description provided.