tc39 / proposal-pattern-matching

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

Design feedback from JSCIG #259

Closed Jack-Works closed 1 year ago

Jack-Works commented 2 years ago

Today I presented the current syntax and semantics to JSCIG and got the following feedback.

Still looks similar to switch

  1. The default keyword (but we cannot use else)
  2. : as the separate token (looks like case x:)
  3. Current design has a strong visual hint (:) that the RHS can be a statement but it actually cannot.

Should nest match shares the same cache?

const it = [0, 1, 2].values()
match (it) {
    when ([a, b, c]): match (it) {
        when ([a, b, c]): void 0 // can we reach here?
    }
}

@@matcher is a terrible name.

And even renaming it to something else is not solving the problem, because we cannot remove @@match and people will keep stuck on it.

How built-in @@matcher work exactally?

e.g. How Error@@matcher work?

Better try-catch?

try {}
catch when (pattern) {}
catch if (expr) {}
catch when (pattern) {}
catch {}
ljharb commented 2 years ago

I agree on default - I'd strongly prefer an alternative.

I don't think : matters because object literals already have that, as does TS types.

: in the language (and in TS) is only followed by a statement with a label, but only an expression everywhere else, and since nobody uses labels, that seems like not something worth worrying about.

ljharb commented 2 years ago

As for "better try-catch", that's only something a followup proposal should do - see #239, #144, #18, #14.

ljharb commented 2 years ago

Error[Symbol.matcher] would check for the presence of the [[ErrorData]] internal slot, I assume.

tabatkins commented 2 years ago

I continue to not have strong opinions on the default name, but we already decided against else due to it visually chaining with a preceding if() matcher. There's not a lot else we can do besides default if we don't have an official nil-matcher. (That is, I think we could remove default if when(_) was an auto-match that didn't generate a binding.)

And yeah, : doesn't suggest a statement anywhere besides switch; it carries an expression everywhere else in the language.

(In general, tho, I don't have a problem with us slightly overlapping with switch syntactically, even if we have differences like this. It's fine, this is a full switch replacement, it's something to learn once and never worry about again.)

Error[Symbol.matcher] would check for the presence of the [[ErrorData]] internal slot, I assume.

Yes, whatever the theoretically equivalent of Error.isError would do, which is gonna be some variety of brand check. The subclasses would brand-check for their specific subclass, etc.

And yeah, I feel strongly that integrating pattern-matching with try/catch is something that would be good to do, but in a follow-up; it's not part of the core syntax and it's got enough complexity that it would drag the main proposal.

tabatkins commented 2 years ago

Should nest match shares the same cache?

Strongly inclined to say no. Using it for anything else in the RHS will show it already advanced an appropriate amount; it seems like it would be really confusing if using it in a match construct seemed to "rewind" it. Fragile, too, since you can do stuff like it.next() && match(it) {...} in the RHS.

noppa commented 2 years ago

I think we could remove default if when(_) was an auto-match that didn't generate a binding

Has this been explored with matchers that would not be valid bindings anyway? E.g. when(else)

match (res) {
  when ({ status: 200 }) { ok(); }
  when ({ status }) if (400 <= status && status < 500) { err(); }
  when (else) { noResponse(); }
}
ljharb commented 2 years ago

We should discuss further, but I think this doesn't need to block stage 2.

fuchsia commented 2 years ago

Another choice of color for the bikeshed:

How about do instead of : And * has plenty of precedent as a wildcard. So it becomes:

match (res) {
  when ({ status: 200 }) do ok();
  when ({ status }) if (400 <= status && status < 500) do err();
  when (*) do noResponse(); 
}

If do-expressions ever arrive, then you can incorporate them.

That avoids setting up semantics for blocks that ends up being different to those used in do-expressions. Hive off the arguments to that proposal.

ljharb commented 2 years ago

This proposal will not advance past do expressions; it depends on them.

Jack-Works commented 1 year ago

close for now because we're redesign things