Union any

[JesterXL]

Allows a catch all Union matcher via the Symbol "any". If you have a big union list, or just don't care and want to catch any of of them (like the default in a switch statement), you can use the new any symbol. References issue #139

const { union } = require('folktale/adt/union'); 
const { any } = union;
const List = union('List', {
  Empty: () => {},
  Cons: (value, rest) => ({ value, rest })
});

List.Cons(1, List.Empty()).matchWith({
  Cons: (x) => 'cons', 
  [any]: (x) => 'anything else'
});

This PR actually undo's the previous by no longer asserting that the matched object have the specific union your looking for. Instead, it'll attempt to find what union was matched first, then the any symbol, and finally will assert an error.

If this is worth merging, I can write up some documentation attempting to following what y'all have done so far with examples.

[JesterXL]

Coverage decreased, nooooOooOooooo... not on my watch, I'll fix that too.

[jreut]

In the default branch of this pattern match, we can't always be sure about which variant we're going to get passed in as this. For example:

const { union, any } = require('folktale/adt/union');

const List = union('List', {
  Cons: (value, rest) => ({ value, rest }),
  Empty: () => {}
});

const addOneToHead = xs => xs.matchWith({
  [any]: ({ value }) => value + 1
});

const listWithHead = Cons(4, Empty());
addOneToHead(listWithHead);
//=> 5

const emptyList = Empty();
addOneToHead(emptyList);
//=> TypeError: Cannot match against 'undefined' or 'null'.

The error here is in the ({ value }) => value + 1 function that tries to destructure a value property from undefined. Usually, the "default" branch of a pattern match does not depend on the variant that happens to have reached that branch. What do you think about a change like this?

diff --git a/packages/base/source/adt/union/union.js b/packages/base/source/adt/union/union.js
index 91e98ab..80a2f5c 100644
--- a/packages/base/source/adt/union/union.js
+++ b/packages/base/source/adt/union/union.js
@@ -110,7 +110,7 @@ instead to check if a value belongs to the ADT variant.`);
       matchWith(pattern) {
         assertObject('Union.mapObject#matchWith', pattern);
         if (name in pattern) { return pattern[name](this); }
-        else if (ANY in pattern) { return pattern[ANY](this) }
+        else if (ANY in pattern) { return pattern[ANY](); }
         else { throw new Error(getMatchWithErrorMessage(pattern, name)) }
       }
     });

[JesterXL]

Now this test is failing:

it('any works with all nested variants', ()=> {
        const { Miss, Hit, Attack } = union('', { 
          Miss() { return { } },
          Hit(damage, critical) { return { damage, critical } },
          Attack(attackResult) { return { attackResult } }
        });

         $ASSERT(
           Attack(Hit(1, false)).matchWith({
            Miss: ()=> 'miss',
            Hit: ({damage, critical}) => 'damage',
            [any]: ({ attackResult }) => attackResult.damage
          })
          == 1
        );
      });

Trying to figure out why; I rewrote the [any] to take just a plain argument, but it's undefined. Is this because the payload was lost without passing in this?

[JesterXL]

... actually, sorry, coffee kicking in, that was point; like you don't care about the arguments, they're supposed to be ignored. Fixing test...

$ASSERT(
           Attack(Hit(1, false)).matchWith({
            Miss: ()=> 'miss',
            Hit: ({damage, critical}) => 'damage',
            [any]: () => 'nothing'
          })
          == 'nothing'
        );

[JesterXL]

This is sad, though, because I envisioned it being like Exlixir's case/cond matchers. It would just forward the results along. I guess with Union it doesn't really make sense, I'm not experienced enough to know. Any ideas how to optionally include the payload would be cool.

[jreut]

@JesterXL I'm moving this little conversation off that diff thread to make it a little easier to follow.

I should also mention that I am not a maintainer of this project. I just happen to use this library and I just started contributing to it.

---

I envisioned it being like Exlixir's case/cond matchers

Could you elaborate a bit on what that sentence means? I'm unfamiliar with Elixir and I'm interested in hearing more about what you're trying to accomplish. I'm not sure I understand what it means to "forward the results along" to the default case.

---

My reasoning for this behavior comes from how I'd write a pattern match in an ML-like language (a language like Elm or Haskell or OCaml):

type Move
  = Miss
  | Hit Int Bool
  | Attack Int

run : Move -> String
run move = case move of
  Miss -> "miss"
  Hit damage isCritical ->
    if isCritical then
      "critical hit for " ++ (show damage)
    else
      "normal hit for " ++ (show damage)
  _ -> "?"

If I tried to write that pattern match differently, say like this:

run move = case move of
  Miss -> "miss"
  Hit damage isCritical ->
    if isCritical then
      "critical hit for " ++ (show damage)
    else
      "normal hit for " ++ (show damage)
  _ attackResult -> "?"

I'll get a syntax error. For instance, the Elm compiler fails like this:

-- SYNTAX PROBLEM ----------------------------------------------------- Main.elm

I ran into something unexpected when parsing your code!

33|         _ attackResult ->
              ^
I am looking for one of the following things:

    an arrow '->'
    whitespace

That's because ML's don't allow you to do anything specific with the variant of the union type once you've decided to handle it in the default case. If there's some specific behavior you want for a particular variant, it's better to explicitly state that by writing a specific case.

😅 That was a little long-winded. Is there anything I could explain a little better?

[JesterXL]

@jreut No worries. I don't know much about functional programming, so I'm learning as I go. I may mis-represent words or concepts by accident, so be forewarned.

Yeah, Elixir's cond and case are dope, check out the first case example and others. iex is like the node repl.

iex> case {1, 2, 3} do
...>   {4, 5, 6} ->
...>     "This clause won't match"
...>   {1, x, 3} ->
...>     "This clause will match and bind x to 2 in this clause"
...>   _ ->
...>     "This clause would match any value"
...> end
"This clause will match and bind x to 2 in this clause"

The union types being matched everywhere has lead to Maybe, Result, and Validators all using the matchWith method, which is really cool and easy to learn. However, in playing with it for a couple weeks it "felt" like the Elixir cond, and so my next immediate thought was "dude, we need a default when you really don't care, I just want a default condition to keep moving" like Elixir has.

Problem is I'm still learning this whole Union type thing, so perhaps it's not related and I shouldn't use it like that.

Naw, your Elm'ish example makes total sense. Maybe I'm longing for Elixir, but Folktale makes my JavaScript job fun, and I'm projecting, lol?

[robotlolita]

The pattern matching features in Elixir (case ... do) and Elm (case ... of) are similar. The major difference is that Elixir is a dynamic language, so the patterns you provide in Elixir don't have to be of the same type as the expression you're matching against, whereas in Elm it's not possible to do something like:

type X = A | B
type Y = C | D

case A of
  A -> "It's A"
  B -> "It's B"
  C -> "It's C" -- this line is an error because `C` isn't part of X

In Elixir you can mix strings and numbers and arrays in your patterns, and that's all cool because the checks are made dynamically.

The way default cases work in either are roughly the same, though. When @jreut suggested removing the parameter for the default case, the reasoning was more in the lines of "We don't know what value is going to be passed there, so we shouldn't pass one." This is because the default case may match more than one type of value.

Consider:

const Move = union('Move', {
  Miss: () => ({ }),
  Hit: (damage, critical) => ({ damage, critical }),
  Attack: (value) => ({ value })
});

Move.Miss().matchWith({
  Hit: ({ damage, critical }) => `${critical ? 'critical' : 'normal'} hit for ${damage}`,
  [any]: ({ value }) => `attack for ${value} damage`
});

Both Attack and Miss may result in the invocation of the default function. In this example, the output is attack for undefined damage because Miss doesn't have a value field. In Elm (and similar language), instead of a struct/record, each of Hit/Attack/Miss would have a different number of positional arguments, so trying to access an index that doesn't exist would result in an actual exception:

type Move =
  Miss -- 0 parameters
  Hit Int Boolean -- 2 parameters
  Attack Int -- 1 parameter

case Miss of
  Hit damage isCritical -> ...
  _ damage -> ...
  -- ^ this is an error because `Miss` has 0 parameters, so there's nothing
  -- to bind to `damage`.

In Folktale's union case, we always pass one object as the only parameter. However, each of these objects will have a different set of properties, so as soon as you try to access these properties/destructure, you'll need to know which properties are available. There's no way of knowing this when you're inside the default case, because it can match any of the alternatives. While there's no actual harm in passing the parameter (unlike in e.g.: Elm), destructuring is a common thing to do, and if we don't pass the parameter that will result in an early error, since you can't destructure undefined.

That said, you'd still be able to access the value inside of the default case if you have a variable pointing to it:

const debug = require('folktale/adt/union/derivations/debug-representation');

const Move = union('Move', {
  Miss: () => ({ }),
  Hit: (damage, critical) => ({ damage, critical }),
  Attack: (value) => ({ value })
}).derive(debug);

function describe(move) {
   return move.matchWith({
    Hit: ({ damage, critical }) => `Hit: ${damage}, ${critical ? 'critical' : 'normal' }`,
    [any]: () => move.toString() // -- safe because all of them implement .toString()
  });
}

describe(Move.Hit(1, true)); // => "Hit: 1, critical"
describe(Move.Miss()); // => "Miss({ })"
describe(Move.Attack(1)); // => "Attack({ value: 1 })"

I... hope that clarifies things. But feel free to ask if any of this is confusing.

[robotlolita]

Ah, this should be ${name}#matchWith instead of 'Union.mapObject#matchWith'.

[JesterXL]

Got it, updating.

[robotlolita]

This is a minor nit-picking, but could you format this with each statement in its own line?

if (name in pattern) {
  return pattern[name](this);
} else if (ANY in pattern) {
  return pattern[ANY]();
} else {
  throw new Error(getMatchWithErrorMessage(pattern, name));
} 

[JesterXL]

sure thing, updating.

[JesterXL]

[robotlolita]

Ah, this should use template strings, as JS doesn't do interpolation on regular ones.

`${name}#matchWith`

[JesterXL]

Whoops... sorry about that.

[JesterXL]

Made the 2 changes requested. Regarding documentation, I couldn't find a link to the union document that you have in the union.md file. I figured I'd add some supplementary union any documentation there in hopefully the same style, etc. Is that ok, or should it go elsewhere?

[robotlolita]

Mmh, which link?

The docs for union are all here https://github.com/origamitower/folktale/blob/master/annotations/source/en/adt/union/union.md, but I just realised it doesn't talk about .matchWith at all :(

If you'd like to add docs to that, it'd be greatly appreciated :) It could go just after the Modelling data with adt/union section.

[JesterXL]

Yup, that's the one. Ok, cool, will do.

[JesterXL]

I've been using Folktale for a month at work in Node, and the hiccup I constantly run into is the mis-match of pattern[name] when you start nesting various Results/Maybes/Validators together and screw up what you thought you'd be getting (i.e. .matchWith of Failure/Success vs. Error/Ok). I can't recall if there was anything we could do to enhance the output of assertObject to maybe help here... beyond using TypeScript/PureScript lulz.

Anyway, here is my first stab at documenting the any symbol after your coverage of matchWith. We're doing a lot of matching against validators and union types at work, and any would really help in those various "20 results all return the same thing" scenarios.

I wrote 5 titles to choose from, but feel free to suggest others for me to use instead.

[robotlolita]

A more general title like the following would be better:

### Matching many cases at once with `any`

[robotlolita]

Instead of:

we can use the `any` type.

It would be more accurate/less confusing to use:

we can use the special value `any`

[robotlolita]

The paragraph should end with a : or . instead of ,

[robotlolita]

Ending this paragraph with :: allows the following piece of code to be used as a test, and forces the docs to be kept in sync if anything in to implementation changes.

Something like the following is enough to turn this into a useful test:

const { any } = require('folktale/adt/union');

function hasAcceptableAuthHeader(authHeader) {
  return authHeader.matchWith({
    Unknown: () => false,
    [any]: () => true
  });
}

hasAcceptableAuthHeader(AuthHeader.Unknown());
// ==> false

hasAcceptableAuthHeader(AuthHeader.JWT());
// ==> true

[robotlolita]

We can change:

assertObject(`${name}#matchWith`, pattern)

To:

assertObject(`${typeId}'s ${name}#matchWith`, pattern);

To output the Union type as well, which should help understanding what kind of thing you've got. Though that's about the most we can do without using something like TypeScript.

[robotlolita]

@JesterXL hey, can I merge this PR?

[JesterXL]

@robotlolita Not yet, lemme work on those changes you requested. Apologies for the delay.

[robotlolita]

No worries :)

[radmen]

@JesterXL what's the status of your PR? I'd fancy use [any] :)

[alewaros]

@JesterXL @robotlolita Any updates on this PR? The overall error handling for pattern[name] is not a function is quite burdensome.

[JesterXL]

I'll try again tomorrow. I just suck at Make, y'all, heh!

[imranismail]

@JesterXL what's the status of this PR?

[robotlolita]

I'm going to merge this, and we can look into improvements later :>

Thanks for all the work, @JesterXL