Open mindplay-dk opened 3 years ago
type for action.type
is 'INCREMENT' | 'DECREMENT'
and also it can be anything? since there is default:
so correct type for action.type
would be 'INCREMENT' | 'DECREMENT' | unknown
which is same as unknown
.
also this snippet fails to type check and produces this <_b: { 'type': _b0, ... }>(number, _b) => number
as type for update
function and _b0
is something that can't be satisfied when calling this function.
type for
action.type
is'INCREMENT' | 'DECREMENT'
and also it can be anything? since there isdefault:
so correct type foraction.type
would be'INCREMENT' | 'DECREMENT' | unknown
which is same asunknown
.
It says default: return state
- so it should be 'INCREMENT' | 'DECREMENT'
, but it's not.
also this snippet fails to type check
That's the point if this issue. It's not clear to me if you're debating or elaborating? 🙂
Inference from usage seems to be about "what could the developer possibly mean" - and in this particular case, it seems less likely you'd want action.type
constrained to a type that the function can actually handle. Otherwise, you'd have declared a (wider) type, right?
That's the point if this issue. It's not clear to me if you're debating or elaborating? 🙂
elaborating
your issue seems to be about inferring type of action.type
to union of string literals but that does not mean it's failing to type check in general
for example if action.type
would be inferred to be unknown
it would type check but not in a way that you wanted
It says
default: return state
- so it should be'INCREMENT' | 'DECREMENT'
, but it's not.
so why do you have default case if only INCREMENT
and DECREMENT
is allowed?
Inference from usage seems to be about "what could the developer possibly mean" - and in this particular case, it seems less likely you'd want
action.type
constrained to a type that the function can actually handle. Otherwise, you'd have declared a (wider) type, right?
that function can handle any object as action
even one without type
property since there is default: return state
so constraining action.type
to something that would make default: return state
unreachable branch would not make sense
It says
default: return state
- so it should be'INCREMENT' | 'DECREMENT'
, but it's not.so why do you have default case if only
INCREMENT
andDECREMENT
is allowed?
Because JavaScript? Type-checking is compile-time, so somebody will always be able to pass anything.
Inference from usage seems to be about "what could the developer possibly mean" - and in this particular case, it seems less likely you'd want
action.type
constrained to a type that the function can actually handle. Otherwise, you'd have declared a (wider) type, right?that function can handle any object as
action
even one withouttype
property since there isdefault: return state
so constraining
action.type
to something that would makedefault: return state
unreachable branch would not make sense
But there's no such thing as an exhaustive switch/case in JS - so no matter what I do, handling default
or not, you can always pass anything to this function. Even if for default
there was a throw
statement, it's still a branch, there's still a control flow, and therefore a type, in this case just an exception.
Reducer patterns like these are super common in JS now - is there some way we can handle them?
Hey, @mindplay-dk. Thank you a lot for the proposal. Have an additional question: how in your case should we handle the next code:
function update(state, action) {
switch (action.type) {
case "INCREMENT":
return state + 1;
case "DECREMENT":
return state - 1;
// case "VALID_BUT_NOT_HANDLED_ACTION":
default:
return state;
}
}
let nextState = update(0, { type: "VALID_BUT_NOT_HANDLED_ACTION" }); // should be a type error here?
Seems like, if a developer will have same logic for some action and default
cases then it needs to add extra runtime code for the valid type inference, so we need a more complex heuristic for this type of inference 😞
Yeah, default
will make the function accept anything.
How about this then?
function update(state, action) {
switch (action.type) {
case "INCREMENT":
return state + 1;
case "DECREMENT":
return state - 1;
default:
throw new Error(`unsupported action: ${action.type}`);
}
}
Or just this:
function update(state, action) {
switch (action.type) {
case "INCREMENT":
return state + 1;
case "DECREMENT":
return state - 1;
}
throw new Error(`unsupported action: ${action.type}`);
}
Any case
not covered by the switch
will generate an exception - effectively removing the possibility of passing any values/types other than those explicitly handled. Would that work?
@mindplay-dk that sounds like a good idea
and would imply that this code
function concat(a, b) {
if (typeof a !== "string") throw new TypeError("a must be a string!");
if (typeof b !== "string") throw new TypeError("b must be a string!");
return a + b;
}
will produce this interface
(string, string) => string
instead of this one
(unknown, unknown) => string throws TypeError
which would be very good for inferring types of js code instead of using poorly implemented .d.ts
files from DefinitelyTyped
Would it possible (and would it make sense) to add inference for pattern matching?
For example:
Here, it's clear from reading the code that
action.type
has a defined set of expected values.Hegel does infer from usage that
action
must have atype
property.But it does not infer that the
type
is a string - although it's clear to a person familiar with pattern matching that, not only is a string property expected, but there is also an expected set of constant string types.I'm sure this is non-trivial, but I'm wondering if it's even possible and whether it would make sense? 🙂