tc39 / proposal-do-expressions

Proposal for `do` expressions
MIT License
1.12k stars 14 forks source link

Nesting do(s) for nested control flow, is this necessary, is there a better way? #47

Closed hodonsky closed 4 years ago

hodonsky commented 4 years ago

I'd much rather use an IIFE here, and yes I do understand the performance implications.

Any thoughts on dropping the required nested 'do' and detecting the statements and expressions independently or would that be too much work on the interpreter?

  fn( item ){
    const id = do {
      switch ( item.constructor ) {
        case Array :
          (do {
            if ( item.length >= 1 && item.length < 3 ){
              (do{
                if ( typeof item[ 0 ] === "string" ){
                  item[ 0 ]

vs

  fn( item ){
    const id = () => {
      switch ( item.constructor ) {
        case Array :
          if ( item.length >= 1 && item.length < 3 ){
            if ( typeof item[ 0 ] === "string" ){
              return item[ 0 ]

I'm throwing specific errors here, so please don't tell me to "just combine your second conditional", I'm asking because there could be other situations I can think of where this could get out of control and make reading code very laborious... even requiring a {> ... } I think would be obnoxious here...

I'm also sure there's probably a good reason it requires nesting, but can it not in the future? Because this would be so very nice:

  fn( item ){
    const id = do {
      switch ( item.constructor ) {
        case Array :
          if ( item.length >= 1 && item.length < 3 ){
            if ( typeof item[ 0 ] === "string" ){
              item[ 0 ]
ljharb commented 4 years ago

Since the entire program could be wrapped in a do expression, if we could do that and wanted to, wed just not require the outer do expressions.

Even if we can, i think the explicit marker at every instance is critical for maintainability.

hodonsky commented 4 years ago

I guess the same could be said about the 'with' keyword... but no one does that, because it's a ridiculous scenario that no one would ever actually implement. You're suggesting because someone can do something stupid with it, we shouldn't add it as a language feature?

hodonsky commented 4 years ago

Also, I like how babel is handling this right now...

_createClass(Checkout, [{
    key: "fn",
    value: function fn(item) {
      var id = function () {
        switch (item.constructor) {
          case Array:
            return function () {
              if (item.length >= 1 && item.length < 3) {
                return function () {
                  if (typeof item[0] === "string") {
                    return item[0];
                  } else {
                    throw new TypeError("Invalid tuple id, 0 index argument must be a String");
                  }
                }();
              } else {
                throw new TypeError("Invalid tuple size, must be length of 1 or 2, 0 index argument must be String(id)");
              }
            }();

So basically I'm going back to a single IIFE...

bakkot commented 4 years ago
  fn( item ){
    const id = do {
      switch ( item.constructor ) {
        case Array :
          if ( item.length >= 1 && item.length < 3 ){
            if ( typeof item[ 0 ] === "string" ){
              item[ 0 ]

That should work fine in the current proposal. Specifically, if I write it out a bit:

function fn( item ){
  const id = do {
    switch ( item.constructor ) {
      case Array :
        if ( item.length >= 1 && item.length < 3 ){
          if ( typeof item[ 0 ] === "string" ){
            item[ 0 ];
            break;
          }
        }
      default:
        null;
    }
  };
  console.log(id);
}

then fn(x) will log x[0] when x is an Array* of 1 or 2 whose first item is a string, and will otherwise log null.

* modulo some pedantry about item.constructor not quite checking that

Why would you expect to need the nested dos?

hodonsky commented 4 years ago

@bakkot Well it doesn't transpile correctly in babel at least... but yeah I thought so too.

It's not an expectation, it's what I had to do to that code to get it to work correctly.

bakkot commented 4 years ago

Babel's do expression transform has a bug, then. Not too surprising, since this is a very early stage proposal.

Edit: opened https://github.com/babel/babel/issues/11608. Would you close this issue, since it seems like the specified behavior is the one you want?

hodonsky commented 4 years ago

@bakkot How helpful... thank you. I thought it might be something like that because they had a switch bug for a while last year I think around June but when I started working with wrapped statements I was like "OH NOOooo is this how it's supposed to be?"

Cool thank you. I will not lose hope in 'do'!

I should report this in babel's plugin then huh?

bakkot commented 4 years ago

I should report this in babel's plugin then huh?

Already done.