tc39 / proposal-pattern-matching

Pattern matching syntax for ECMAScript
https://tc39.es/proposal-pattern-matching/
MIT License
5.5k stars 89 forks source link

Maybe JS doesn't really need pattern matching #301

Closed theScottyJam closed 7 months ago

theScottyJam commented 1 year ago

I know I'm posting this issue on a repo full of people who would love to see this proposal go in, so I'm sure I'll get a ton of dislikes for bringing this issue up :). But I think it's an important conversation to have.

Does pattern matching pay for itself?

I'm generally fond of pattern-matching syntax, especially in functional languages. There's a certain cleanliness to it. It's nice to see many languages starting to adapt various functional features such as this.

That being said, I've recently been pondering the concerns many people have about JavaScript's syntax getting too large, and I can't help but think about pattern matching and the problems its trying to solve vs the amount of syntax it's using to solve those problems. I'm becoming increasingly convinced that pattern matching may not be a good fit for JavaScript - it works well in languages that were built from the ground-up with pattern-matching in mind, but when it's tacked on after-the-fact, it just doesn't pay for itself. To expound on that last point a bit - most functional languages don't have a separate syntax for destructuring and pattern matching, you just destructure via pattern matching. The fact that JavaScript will have to have both means it needs to bring in much more syntax, semantics, and most importantly, conceptual overhead to pay for this feature. (and yes, I know pattern-matching tries to be a superset of destructuring, which helps, but it still deviates in commonly used scenarios, e.g. pattern matching with array syntax does a length check, but you can destructure an array of any length no problem)

How much does pattern matching really improve your code?

I'm going to do a side-by-side comparison of some of the motivating examples this proposal provides, along with how those same examples could be written without pattern matching. While I don't feel it's strictly necessary, I will make use of a couple of other smaller proposals as I make this comparison:

  1. Do expressions
  2. a noelse "proposal" I'm making up right now. If you put "noelse" at the end if an if-chain, then an error will be thrown if none of the if/else-if blocks were entered.
// with pattern matching

const res = await fetch(jsonService)
match (res) {
  when ({ status: 200, headers: { 'Content-Length': s } }):
    console.log(`size is ${s}`);
  when ({ status: 404 }):
    console.log('JSON not found');
  when ({ status }) if (status >= 400): do {
    throw new RequestError(res);
  }
};

// without pattern matching

const res = await fetch(jsonService)
if (res.status === 200) {
  const s = res.headers['Content-Length'];
  console.log(`size is ${s}`);
} else if (res.status === 404) {
  console.log('JSON not found');
} else if (res.status >= 400) {
  throw new RequestError(res);
} noelse;

// -------------------- //

// with pattern matching

function todosReducer(state = initialState, action) {
  return match (action) {
    when ({ type: 'set-visibility-filter', payload: visFilter }):
      { ...state, visFilter }
    when ({ type: 'add-todo', payload: text }):
      { ...state, todos: [...state.todos, { text, completed: false }] }
    when ({ type: 'toggle-todo', payload: index }): do {
      const newTodos = state.todos.map((todo, i) => {
        return i !== index ? todo : {
          ...todo,
          completed: !todo.completed
        };
      });

      ({
        ...state,
        todos: newTodos,
      });
    }
    default: state // ignore unknown actions
  }
}

// without pattern matching

function todosReducer(state = initialState, action) {
  return do {
    if (action.type === 'set-visibility-filter') {
      const visFilter = action.payload;
      ({ ...state, visFilter });
    } else if (action.type === 'add-todo') {
      const text = action.payload;
      ({ ...state, todos: [...state.todos, { text, completed: false }] });
    } else if (action.type === 'toggle-todo') {
      const index = action.payload;
      const newTodos = state.todos.map((todo, i) => {
        return i !== index ? todo : {
          ...todo,
          completed: !todo.completed
        };
      });

      ({
        ...state,
        todos: newTodos,
      });
    } else {
      state; // ignore unknown actions
    }
  }
}

// -------------------- //

// with pattern matching

<Fetch url={API_URL}>
  {props => match (props) {
    when ({ loading }): <Loading />
    when ({ error }): do {
      console.err("something bad happened");
      <Error error={error} />
    }
    when ({ data }): <Page data={data} />
  }}
</Fetch>

// without pattern matching

<Fetch url={API_URL}>
  {props => do {
    if ('loading' in props) {
      <Loading />
    } else if ('error' in props) {
      console.err("something bad happened");
      <Error error={error} />
    } else if ('data' in props) {
      <Page data={data} />
    } noelse
  }}
</Fetch>

Did pattern matching improve the quality of these examples? Absolutely.

Did it improve them by a ton? Not really. The code is still fairly easy to read in either version.

Now considering the size of the pattern-matching proposal and the huge quantity of syntax it's adding, you can (hopefully) see why I'm starting to get skeptical about it.

But what about...?

There are other use-cases that pattern-matching brings to the table, that I'd like to briefly discuss.

Pattern matching is supposed to be a better replacement for "switch", and while it would be nice to have a good "switch" replacement, honestly, "switch" probably didn't need to exist in the first place either. Using if/else works good enough. The syntax cost for "switch" probably didn't pay for itself either.

Pattern-matching also shines with certain types of patterns. Take, for example, a pattern like this:

match (value) {
  when ({ aReallyLongPropertyName: { x: 0, y: 0, z: 0 }}): do {
    ...
  }
}

A brute-force conversion of this code sample would be fairly ugly.

if (
  'aReallyLongPropertyName' in value &&
  value.aReallyLongPropertyName.x === 0 &&
  value.aReallyLongPropertyName.y === 0 &&
  value.aReallyLongPropertyName.z === 0
) {
  ...
} else if ...

But, with a little bit of thought, it's not that hard to clean up this sort of thing.

const maybeCoord = value.aReallyLongPropertyName;
if (maybeCoord?.x === 0 && maybeCoord?.y === 0 && maybeCoord?.z === 0) {
  ...
} else if ...

The pattern-matching solution is nicer, but the alternative isn't bad or unreadable either.

There may be other pathelogical patterns you can think of where you'll find that pattern-matching can really shine against any other alternatives. But, if we focus on the common day-to-day use of pattern-matching, I'm sure we'll find that most uses can be fairly easily replaced with "if/else" and maybe "do expressions", without much of a readability cost.

Conclusion

I think my opinions are summed up pretty well in the above blurb.

I'm wondering if others have reasons they love pattern-matching that I didn't touch on here. For some of these "loved features", I wonder if there's other smaller proposals that could be introduced that would accomplish the same objective in a simpler way (similar to the noelse idea I tossed around earlier). I do feel like, with this proposal, we started with the solution "let's add pattern-matching syntax" as opposed to a problem statement like "lets fix switch's pitfalls" or "checking many fields of an object/array can be verbose, can we find a more concise way to handle it?"

vendethiel commented 1 year ago

Your examples forego checking nulness of res (so you'd either have an enclosing if, or use ?. everywhere). They also forego checking key presence (You'll still try to access res.headers['Content-Length'] even if that key doesn't exist).

The aReallyLongPropertyName example also works fine with maybeCoord, but this is a very simple case: if you have a lot of different branches, you'd need several such temporaries, and depending on shape all of these might have to use ?..

mAAdhaTTah commented 1 year ago

The big thing this doesn't cover is custom matchers, which isn't easily replicated with switch or if/else. Being able bundle up a type with a mechanism for matching against it is really valuable for functional style or even just handling errors in general. For example, the fetch example can use pattern matching to map the API response to explicit errors instances which can themselves be pattern matched elsewhere to display different messages depending upon the specific response without needing to pass around the raw response object, limiting the API surface that the rest of the application is exposed to.

The other piece is being able to use patterns in if's (which I believe is in the proposal now). Being able to do:

if (value is { aReallyLongPropertyName: { x: 0, y: 0, z: 0 }}) ...

is pretty nice too.

bas080 commented 1 year ago

I'm also on the fence. Pattern matching syntax has implicit behavior when walking nested objects and equality checks. By baking it into the syntax it is unlikely to be configurable.

I've built a pattern matching library and in the process of doing that I am considering offering configurability when it comes to tree traversal and equality checking.

One would have to be quite confident and assured that the decided upon behavior applies to most cases, especially because the behavior is non-configurable.

ljharb commented 1 year ago

This proposal is configurable via the custom matcher protocol.

theScottyJam commented 1 year ago

@vendethiel

Your examples forego checking nulness of res (so you'd either have an enclosing if, or use ?. everywhere).

This is correct. Based on context I left that out - fetch() doesn't return a nullish value so there was no need to check for that. But, you're right, if this was needed, you'd have to do something like put ?. everywhere, which, for me is fine.

They also forego checking key presence (You'll still try to access res.headers['Content-Length'] even if that key doesn't exist).

Yeah, good catch, so if that check for the existence of Content-Length was actually needed, the if() version would need to add that explicit check in as well. (Though, an argument could be made in this specific scenario that we can trust that the API will always return a Content-Length header, so there's not much need for us to explicitly test for it. Of course, other scenarios would be different).

@mAAdhaTTah

The big thing this doesn't cover is custom matchers, which isn't easily replicated with switch or if/else. Being able bundle up a type with a mechanism for matching against it is really valuable for functional style or even just handling errors in general.

There are, however, other ways to achieve something to this effect that don't require a proposal this large. Take, for example, the early declarations in conditionals proposal, and lets also lift the tentative restriction that prevents you from doing destructuring during the assignment. Now you can do this:

const userOrError = await fetchUser(userId);

if (const { username, email } = either.isRight(userOrError)) {
  ...
} else if (const { errorMessage } = either.isLeft(userOrError)) {
  ...
} noelse

I'm actually someone who heavily uses the "return your errors" pattern of programming, and I do see pattern matching as something that would help my particular programming style. Except, it doesn't mesh too well with me also liking to do early returns. (edit: ignore that - I had changed my mind and tried to edit this thought out but missed a spot) e.g. I'm often writing code like this today:

const didOperationOk = await doOperation();
if (didOperationOk.type === 'forbidden') {
  return { type: 'forbidden' };
} else if (didOperationOk !== 'ok') {
  throw new UnreachableCaseError(didOperationOk);
}
const { value: result } = didOperationOk;

...rest of function...

return { type: 'ok' };

(The UnreachableCaseError is a special error class I use to hint to TypeScript that I don't want it to allow this code to be reached. It takes a never type as a parameter. See here)

I won't refactor this in the various ways with the different proposals, but if I were to do so, the results would be somewhat similar-feeling to the other examples I already compared between.

bathos commented 1 year ago

I appreciate that this thread was opened. I’ve found myself with the same reservations despite admiring the premise. Those reservations have grown as the proposal’s complexity expanded. I don’t understand how the density of novel syntactic constructs and (relative to existing ECMAScript syntax and semantics) idiosyncratic patterns is justified.

What is or isn’t justified is a subjective assessment, though, and the proposal is still in development. I certainly wouldn’t intend or expect to convince anyone involved that the proposal should be dropped. However it would be nice to know if these concerns are shared by its champions and if so, whether that’s likely to influence further development of the proposed design.

ljharb commented 1 year ago

These concerns are always shared by many in the committee with any syntax proposal, and don’t need to be brought up to be considered.

theScottyJam commented 1 year ago

Hmm, perhaps part of what contributes to the bloated feeling is the fact that this proposal does seem to carry a lot of features that don't seem essential for an initial pass. maybe if it was paired down I would be more comfortable with it.

As it currently stands, I feel like this proposal is trying to be flexible enough to handle any possible use-case one might have in a pattern position so that no one would ever feel the need to fall back to some other technique (e.g. a normal if/else) if some complicated scenario comes up. Perhaps it would be more healthy to, for now, frame this proposal in terms of "what is the minimum amount of syntax needed to handle the majority of use cases", and then be ok with people needing to occasionally use if/else or something else for odd-ball situations.

Where things went wrong

I feel like many of the complexities of this proposal snowballed from regular expressions. We start with the idea of "wouldn't it be neat if regular expressions could be used as patterns", which yes, it would be nice. But wait, what if you also want to bind to the result of the regular expression's match? We'll need to support some sort of "with" syntax to support that use-case. But why limit that feature to regular expressions? Shouldn't you be able to do the same sort of thing with userland classes? So now we have a matchable protocol. The matchable protocol also enables other kinds of nice custom patterns, like "is this greater than zero", or "is this nullish", or whatever you might dream of. But now we have a new problem - what if we want to compose multiple custom patterns together? Well, now we need the "and" operator. And for feature parity, "or" and "not" would be good to add as well.

Perhaps we're starting to loose sight of the core problem being solved?

Just the core

What if we just dropped most of that? The outer pattern-matching control structure can stay the same (when, if guards, and default are all there, just the same), but the actual pattern syntax can be greatly paired down to just array, object, primitives, and bindings - that's it. Perhaps we don't even support regular expression patterns - they're kind of neat, but they're not really needed to support the core functionality.

What if you need to do some custom logic (e.g. userland custom matchers)? Just call functions in your guards instead - it's no different from what you would have to do today, plus, now we're not asking library authors to provide various custom matcher protocols to make their library more user-friendly.

What if you need to check if a value is one of multiple things (e.g. the "and" combinator)? e.g. you can let it either be the string "disabled" or "off"? Do ['disabled', 'off'].includes(yourBinding) in your guard.

What if you want custom logic that allows you to introduce bindings and be capable of making this particular match arm fail (e.g. the "with" operator-thing)? Drop the pattern matching. Your logic is probably better represented as a series of "if"s with early returns (if an example is needed to illustrate what I mean here, I can clobber something together).

Misc thoughts

I was just looking through one of my projects, searching for places where it would make sense to introduce pattern matching, and I found quite a few places where it would work well. The vast majority were places where the patterns would be extremely simple - things like when ({ type: 'accessProperty', propertyKey }): ... or when ([key, ...remainingKeys]): .... Very simple use-cases that only require the core features of pattern matching. I also found a handful of scenarios that could be converted to use pattern matching if I used some of the more advance syntax it offered, but honestly, in these cases I found, the code would have been about just as readable in either format. Pattern matching seems to shine the strongest when you're dealing with very simple patterns. So perhaps it makes sense to limit the scope of this proposal to just dealing with simple patterns - anything too complicated is simply out-of-scope, people will have to use a different tool.

Maybe in the future we can look more into adding some of these other features like custom matchers, and/or, or I know the proposal is about to go through a large update where things like extractor patterns will now take a front row seat. Or maybe those don't ever get added in. This proposal could, perhaps, stand just fine without all of that stuff.

rkirsling commented 1 year ago

And for feature parity, "or" and "not" would be good to add as well.

Note that, while and is indeed conceptually complex, or is a fundamental feature of pattern matching: even if one were imagining this feature as a direct extension of switch, you'd want to be able to write e.g. case '-' | '_':.

ljharb commented 1 year ago

@theScottyJam the proposal hasn’t been updated yet to our newest design - see #293.

ljharb commented 1 year ago

But, again, anything that fails to actively discourage future use of switch isn’t worth adding to the language imo - so “extending switch” is an instant nonstarter for me.

rkirsling commented 1 year ago

so “extending switch” is an instant nonstarter for me.

I don't think anyone in this thread proposed doing so.

mAAdhaTTah commented 1 year ago

I'll just throw out that even the example here is a little messy, including the magic strings of the type:

const didOperationOk = await doOperation();
if (didOperationOk.type === 'forbidden') {
  return { type: 'forbidden' };
} else if (didOperationOk !== 'ok') {
  throw new UnreachableCaseError(didOperationOk);
}
const { value: result } = didOperationOk;

...rest of function...

return { type: 'ok' };

Pattern-matched version would be something like (not fully familiar with the current proposal so may not be quite right):

const didOperationOk = await doOperation();
match(didOperationOk) {
  when(${ForbiddenError}): ... handle ...
  when(${UnexpectedError}): ... handle ...
  default: ... rest of function ...
}

The longer if/else block with the magic type string is both harder to get right as you're writing the code as well as harder to understand when you're reading it later. Bundling all this up into ForbiddenError with a custom matcher is really clean and easy to understand.

Looking at the declarations in conditionals README, it reads:

destructuring is not allowed (even if only a single variable is "pulled out"), as there's potential confusion as to what's actually being checked (the value vs. whether the desired key/index was present in the object/array)

So your example wouldn't work. It's also honestly a bit weird to have the destructuring itself be conditional on the function returning a truthy value and... not do the destructuring if it's falsey. So I don't think that's a potential solution.

tabatkins commented 1 year ago

As multiple people have pointed out, existing alternatives that do the same thing as pattern matching even in relatively simple cases are actually rather verbose and error-prone, and not in ways that can be easily hand-waved as being unimportant. Patterns capture a lot of test patterns that you'd want to do in a super-concise way.

Your central thesis in the OP, that functional languages that start with pattern matching are good but languages that add pattern matching later are bad, is also something I would disagree with vehemently. Any feature added later in a language's life might have some mismatches and warts relative to a theoretical version that was integrated earlier, but across many langs I don't see a very significant difference in the two cases, and the warts in JS's specific case are small and tolerable relative to the benefit, imo.

Importantly, several of your complaints are about the syntax as it currently stands in the README, and many of these complaints are likely addressed in the revisions we're working on in a PR currently. Wait a little bit longer and we'll have a better version up. ^_^

bathos commented 1 year ago

Importantly, several of your complaints are about the syntax as it currently stands in the README, and many of these complaints are likely addressed in the revisions we're working on in a PR currently. Wait a little bit longer and we'll have a better version up. ^_^

I’m glad to hear this, looking forward to it!