tc39 / proposal-pattern-matching

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

Alternative syntax for custom matchers #252

Closed Jack-Works closed 2 years ago

Jack-Works commented 2 years ago

@rbuckton suggests an alternative syntax/proposal called "unapply" which also appears in many other languages.

Current:

match ("#00ffbb") {
    when (${RGB} with [a, b, c]): console.log(a, b, c)
}

unapply:

match ("#00ffbb") {
    when (RGB(a, b, c)): console.log(a, b, c)
}

// also in deconstructing
const RGB(a, b, c) = "#00ffbb"

I personally strongly prefer the unapply style.

ljharb commented 2 years ago

I'm very skeptical of this syntax as well as the entire unapply proposal (which is tough to grok without a repo).

Either way, I don't think it makes sense to radically change a proposal that's ready for stage 2, to be blocked on a proposal that's not even at stage 1 yet.

The name of Symbol.matcher is definitely not final, and because of its similarity with Symbol.match, we'd likely change it during stage 2.

Jack-Works commented 2 years ago

I'm very skeptical of this syntax as well as the entire unapply proposal (which is tough to grok without a repo).

Pattern matching and unapply are of a similar origin (functional programming languages?) so I think they fit well. If we can have unapply, we can avoid overloading of ${} syntax (now it have 2 functionality, matching the value and calling the Symbol.matcher).

If we choose unapply, we can add it to this proposal.

Both destructuring and pattern matching should remain in sync, so enhancements to one would need to work for the other.

ljharb commented 2 years ago

Just because they both exist in other languages doesn't mean they both should exist in JS.

Separately, if we go down this route, then this proposal would be utterly stalled until a separate unapply proposal at least reached stage 2.

Jack-Works commented 2 years ago

Separately, if we go down this route, then this proposal would be utterly stalled until a separate unapply proposal at least reached stage 2.

Not necessary. We can merge it into this repo.

ljharb commented 2 years ago

We already got pushback today about this proposal being too large as it is. Changing existing destructuring syntax is a massive change that could not and should not be merged with pattern matching, an entirely new feature.

theScottyJam commented 2 years ago

I'm not exactly sure how unapply would replace custom matchers. Wouldn't it be an additional thing to go along with custom matchers?

Custom matchers were original inspired by the need to let userland do what we're doing with regular expressions, which is to match against a regex, and bind values at the same time. How would this use case work with unapply syntax?

i.e. with custom matchers, I can write this:

when (${new RegExp('/ab(c*)/')} with [aBunchOfCs]): ...

And, if JavaScript didn't natively provide regular expressions, someone could invent a regular expression class that provided that exact API. How would this sort of thing look like if we were to use unapply instead?

Jack-Works commented 2 years ago

Maybe

when (${new RegExp('/ab(c*)/')}(aBunchOfCs)): ...

😂

ljharb commented 2 years ago

making a bunch of things look callable doesn't seem like an improvement.

theScottyJam commented 2 years ago

Yeah...

From what I understand, unapply is intended as a syntax that's intended to mirror construction syntax, but instead of constructing an instance, you're pulling out the values that were provided during construction and binding them. Using it for anything else would be an abuse of the mental model it provides, and an abuse of the syntax. with is much more general purpose, and allows you to create custom matchers that behave in any way.

For this reason, I do think it's important that we at least provide a with syntax, and if wanted we can later consider adding unapply syntax as well.

But, I too have similar concerns with unapply. It's primary use case is to extract values that were provided during construction - these sorts of values would generally be found as public properties on the object instead, so you can trivially accomplish the same thing, even without the with syntax by simply doing this:

// Instead of this:
when (Point(x, y)): ...

// Do this:
when ({ x, y }): ...

// Or, if they provide a matcher protocol on the class to let you check if something
// is an instance of the class, you can do this:
when (${Point} and { x, y }): ...

Anyways, these thoughts are more of a discussion for the unapply proposal, but it shows my general concern around it, which is that it burdens API makers to provide a protocol implementation, just to allow end-users to have this feature that really doesn't do much extra over what they can already do.

Jack-Works commented 2 years ago

That's convince me 🤔 but I want to keep this issue open for further discussions

tabatkins commented 2 years ago

In addition to the higher-level concern that Jordan talked about (we don't want to try and invent a new destructuring syntax as part of this proposal), @theScottyJam basically laid out my concerns as well - having reviewed unapply and similar functionalities in several languages, either the syntax is meant to extract values set in the constructor (which, in JS, are virtually always exposed as properties and thus already extractable), or it can be used for completely unrelated purposes and the constructor-looking syntax is somewhat misleading and weird.

Open to being convinced about this later, but for now I'm mildly against doing anything in the "unapply" space here.

tabatkins commented 2 years ago

And note that, if neither of @theScottyJam's options quite work ({x, y} or ${Point} and {x,y}) because the class doesn't expose those values as properties, ${Point} with {x,y} is always available since the custom matcher can return whatever it wants. In particular, if there are multiple possible sorts of data that might be extractable, ${Point.asXY} with {x,y} is possible as well - several more or less identical matchers that just return different value keys in the result.

ljharb commented 2 years ago

Closing for now; if "unapply" ever advances to a stage that matches or surpasses this proposal, we'll certainly re-evaluate.