Skip to content
This repository has been archived by the owner on Jan 29, 2024. It is now read-only.

Inference from pattern matching #344

Open
mindplay-dk opened this issue Aug 9, 2021 · 7 comments
Open

Inference from pattern matching #344

mindplay-dk opened this issue Aug 9, 2021 · 7 comments
Labels
question Further information is requested

Comments

@mindplay-dk
Copy link

Would it possible (and would it make sense) to add inference for pattern matching?

For example:

function update(state, action) {
  switch (action.type) {
    case "INCREMENT":
      return state + 1;
    case "DECREMENT":
      return state - 1;
    default:
      return state;
  }
}

let nextState = update(0, { type: "INCREMENT" });

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 a type 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? 🙂

@thecotne
Copy link
Contributor

thecotne commented Aug 9, 2021

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.

@mindplay-dk
Copy link
Author

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.

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?

@thecotne
Copy link
Contributor

thecotne commented Aug 9, 2021

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

@mindplay-dk
Copy link
Author

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?

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

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?

@JSMonk
Copy link
Owner

JSMonk commented Aug 10, 2021

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 😞

@mindplay-dk
Copy link
Author

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?

@thecotne
Copy link
Contributor

@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

@JSMonk JSMonk added the question Further information is requested label Aug 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants