Proposal-pattern-matching: Question about Todo example

Created on 20 Mar 2019  路  12Comments  路  Source: tc39/proposal-pattern-matching

When looking at the README, it makes me wonder something.

Does this:

when {type: 'set-visibility-filter', filter: visFilter} ->

Have the same meaning as this?

when {type: SET_VISIBILITY_FILTER, filter: visFilter} ->

Or does it attempt to assign type to SET_VISIBILITY_FILTER instead of match it?

If the two examples don't match, that could be a potential foot-gun problem.

question

Most helpful comment

Here's an example based on a random thought. Let's say we add a walrus operator:

let {type: 'set-visibility-filter', filter: visFilter} -> ...
let {type := SET_VISIBILITY_FILTER, filter: visFilter} -> ...

This would essentially be a nested shorthand for a subset of guards. And it would add a new operator for people to learn. I'm strongly leaning towards retaining the current semantics, because while there's an initial gotcha, it's a common gotcha that other languages (Elixir and Rust, for example) have succeeded very well with, so it can't be _that_ much of a gotcha.

I consider this a bit of a syntax micro-optimization and I don't think it adds very much value to the current spec, which I'm trying to keep as clean and simple as I can.

All 12 comments

You are correct that these do not behave the same, and the latter actually _assigns_ SET_VISIBILITY_FILTER. This is existing behavior in Elixir, and works well enough for them. This seems like a good motivation to add variable-matching to the syntax, either through a pinning operator (^SET_VISIBILITY_FILTER), or through paren wrapping (type: (SET_VISIBILITY_FILTER)).

What makes sense in Elixir doesn't necessarily make sense in JavaScript. I think what would make the most sense is to not assign/define variables within the matching syntax, making the two behave the same. Doing so, any of the following would accomplish the same result:

when {type: 'set-visibility-filter'} ->
      return {...state, visFilter: action.filter}
when {type: SET_VISIBILITY_FILTER} ->
      return {...state, visFilter: action.filter}



md5-a479fb61ab6bba2ae2e86072a6118e8a



when {type: SET_VISIBILITY_FILTER} -> {
      const {filter: visVilter} = action 
      return {...state, visFilter}
}

The problem here is what happens if SET_VISIBILITY_FILTER is undefined? You lose destructuring. The variable-binding semantics are pretty core to this proposal.

I thought this was an interesting question, and it does seem like the kind of thing that would cause problem for people in practice. (Though, I guess linting rules about shadowing variables with the same name would be helpful!)

I did have a thought when I saw this issue earlier; I'm not sure this is the way I'd go, but I figure it's worth posting for discussion's sake...

The existing destructuring bindings solve a similar-ish problem, where you can both bind under a different name/destructure further (via { foo: bar }) as well as assign a default value to the destructuring (via { foo = bar }, for any expression bar).

You could take a similar approach with the pattern matching: split the syntax into variable-binding (omit or use colon syntax, to match destructuring) and "field has value" constraints. So, for the examples above, the syntax might be something like

when {type is 'set-visibility-filter', filter: visFilter} -> ...
when {type is SET_VISIBILITY_FILTER, filter: visFilter} -> ...

and if you want to match/extract something deeper down you'd have

when {type is 'get-user', payload: { id, name: username, type is 'admin' }} -> ...

which would put id and username in scope.

This would probably mean bikeshedding the "field constraint" syntax, though, I suppose.

@zkat

If SET_VISIBILITY_FILTER is undefined, wouldn't there be an is not defined error? If it was literally set to undefined, the matcher could match if "type" in action && action.type === undefined.

The issue I am seeing is that the syntax is both destructuring and object creation. It essentially does this:

const pattern = {type: 'set-visibility-filter'} 
const {filter: visFilter} = action

This is confusing because the syntax is ambiguous. It tries to do two different things at the same time: Pattern matching and destructuring. IMO, trying to do destructuring provides little benefit because you can destructure in the matching body. Or in the case of the example, simply do visFilter: action.filter inline on the return.

The only remaining issue I see is this example:

when { x, y, z }

This is valid object creation syntax, assuming x, y, and z are defined. But in this case we don't want them to be. We essentially want this:

const KEY_EXISTS = Symbol()
const pattern = {x: KEY_EXISTS, y: KEY_EXISTS, z: KEY_EXISTS}

Which begs the question: What happens when x, y, and/or z is defined? Does the object property short-hand not apply here? If not, that is a bit easier to swallow. You could require when {x: x, y: y, z: z} if you wanted to match those values. But still, it has some inconsistency.

Then there's this example that mixes destructuring/object creation with if to do something other than ===:

when {status} if (status >= 400) -> {

I wonder if there is a way to accomplish pattern matching with a single syntax than could accommodate cases other than ===.

@FireyFly The thought of using { foo = bar } is an interesting one. I wonder if you could do something along the lines of:

when {type = 'set-visibility-filter'} ->
when {type = SET_VISIBILITY_FILTER} ->

Or perhaps:

when {type === 'set-visibility-filter'} ->
when {type === SET_VISIBILITY_FILTER} ->

And perhaps something like this:

when {status >= 400} -> {

Although it gets more complex when needing || vs &&. Maybe too much? Not sure. The more complex it gets, the more I ask myself "why not if / else if / else?".

I did consider foo = bar originally, but I feel personally that having the same syntax mean "test for equality/identity" in one pattern-match-y context and "assign default value" in another is probably risky/prone to confusion.

I also considered == briefly, but to me that looks too much like "any boolean expression works here", and I'd wonder if I could also match 'set-visibility-filter' == type or field1 == field2 then.

But as mentioned, that syntax could probably be bikeshed. :) The main point is to separate the syntaxes for "bind field to variable" and "constrain match to field having value"

Edit: and yeah, in reply to your most recent comment, I wouldn't really want terribly complicated constraint syntax for field values I think. Better to keep the syntax fairly simple and orthogonal but powerful, IMO.

The main point is to separate the syntaxes for "bind field to variable" and "constrain match to field having value"

Agreed.

I wouldn't really want terribly complicated constraint syntax for field values I think. Better to keep the syntax fairly simple and orthogonal but powerful, IMO.

Going along with that, it should solve pain points faced in production code. How do you do that while hitting all of those (simple, orthogonal, powerful) at the same time?

Perhaps I'm not being fair, but with the TODO example, I'm not sure how it does this. If I were to ask myself honestly which of the two I would prefer: The new pattern matching example vs the original code with the switch... I would say the switch. It's simple. It matches on one key. And the original example returns in each case, so there are no breaks.

IMO, for pattern matching to be useful, it should do these things (while hitting the above points):

1) matching more than one key
2) doing more than ===

I'm not sure it currently does this (At least the "simple" part).

The most compelling example in the README for me is this:

when { x, y, z }

It's compelling because it essentially matches the shape of an object. Match if object has x, y, and z. You can do this with instanceof with classes, but being able to match on plain objects in this way is rather awesome. Although it would be cool if this was an operator like instanceof, so you could do something like this:

if (vector shapeof {x, y, z}) {}

Perhaps a bit off-topic, but if there were shortcomings of a switch that I wish I could overcome (other than auto-breaking; that's a given), it would be that I wish I could do these things with a switch-case:

  case >= 400
  case instanceof Foo

The thought here is that when I choose a switch over if/else if/else blocks, I do so because the switch is DRY. If I could think of a cases to expand switch-case to avoid if/else if/else, those would be it. In a similar vein when I'm looking at pattern-matching, I'm asking myself _why_ would I use it over if/else if/else or switch?

There are some that would say that this is unrelated to switch. But I would say that pattern matching can be a replacement for switch, inasmuch that switch can be a replacement for if/else if/else. You have to consider how they all relate to each other. Pattern matching cannot be designed in isolation, but rather in the context of how it relates to everything else.

Since we're adopting #116 (match expressions), I think I'd write the todo example something like


(Code snippet)

const toggleDoneAt = (todos, toggleIndex) =>
  todos.map((todo, idx) => (
    idx === toggleIndex
      ? {...todo, done: !todo.done}
      : todo
  ));

const todoApp = (state = initialState, action) =>
  case (action) {
    when { type is 'set-visibility-filter', filter } ->
      { ...state, visFilter: filter };
    when { type is 'add-todo', text } ->
      { ...state, todos: [...state.todos, { text }] };
    when { type is 'toggle-todo', index } ->
      { ...state, todos: toggleDoneAt(state.todos, index) };
    when _ ->
      state;
  };

(using is for match constraints as with the suggestion I gave earlier; using an arrow function for todoApp; using the expression form of the match expression; extracting toggleDoneAt)

which I find quite pleasing and easy to understand, personally (subjective, of course).

The language already has destructuring syntax, which allows for array and object destructuring, nesting of the same, extracting the rest of an object/array, and binding a field under a different name. To me, the pattern-matching just adds the ability to match against a field having a specific value, and a surrounding syntax for this to make sense for a match to succeed/fail. To me, this is a small step that adds a lot of power in the matching you can do.


I think we should also decide if this issue is a general "discussion around Todo example", or specifically about addressing the potential problems of variable-binding and field constraints sharing the same syntax (i.e. that type: SET_VISIBILITY_FILTER might work unexpectedly). Either way is fine, though I think a focused discussion on that potential problem would be useful! (Here, or as a separate issue)

Here's an example based on a random thought. Let's say we add a walrus operator:

let {type: 'set-visibility-filter', filter: visFilter} -> ...
let {type := SET_VISIBILITY_FILTER, filter: visFilter} -> ...

This would essentially be a nested shorthand for a subset of guards. And it would add a new operator for people to learn. I'm strongly leaning towards retaining the current semantics, because while there's an initial gotcha, it's a common gotcha that other languages (Elixir and Rust, for example) have succeeded very well with, so it can't be _that_ much of a gotcha.

I consider this a bit of a syntax micro-optimization and I don't think it adds very much value to the current spec, which I'm trying to keep as clean and simple as I can.

I don't see a problem here at all, because we have if on cases

when {type, filter: visFilter} if (type === SET_VISIBILITY_FILTER) ->

The proposal has been updated in #174. If this issue is still relevant, please file a new issue, taking into account the updated state of the proposal.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

elijahdorman picture elijahdorman  路  6Comments

tabatkins picture tabatkins  路  4Comments

Haroenv picture Haroenv  路  6Comments

arcanis picture arcanis  路  6Comments

michaelficarra picture michaelficarra  路  8Comments