MatAtBread / fast-async

605 stars 21 forks source link

Destructuring still broken? #35

Closed djMax closed 7 years ago

djMax commented 7 years ago
      const { body: responseBody, status } = customerSearchResults;
      const { _embedded = {} } = responseBody;
      const { customers = [] } = _embedded || {};
      const [customer] = customers || [];
      const { id: customerId } = customer || {};

I end up with an error saying _embedded is not defined (probably on line 3 there if the sourcemap is to be believed).

It transpiles to this:

              ({ body: responseBody, status } = customerSearchResults);
              ({ _embedded = {} } = responseBody);
              ({ customers = [] } = _embedded || {});
              [customer] = customers || [];
              ({ id: customerId } = customer || {});

Which doesn't look wrong, but somehow ends up failing, perhaps because of the 'use strict'?

MatAtBread commented 7 years ago

Can you provide the surrounding async/await usage, and `use strict' so I can try to reproduce the issue?

djMax commented 7 years ago
async function fn(customerSearchResults) {
  const { body: responseBody, status } = customerSearchResults;
  const { _embedded = {} } = responseBody;
  const { customers = [] } = _embedded || {};
  const [customer] = customers || [];
  const { id: customerId } = customer || {};
  console.log('The answer is', customerId);
}

fn({});
MatAtBread commented 7 years ago

The following test actually works...

async function z() {
    await Promise.resolve(0);
    const customerSearchResults = {
        body: {
            _embedded: {
                customers: [{
                    id: "a"
                },{
                    id: "b"
                }]
            }
        },
        status: "status"
    };
    const {body: responseBody, status} = customerSearchResults;
    const {_embedded = {}} = responseBody;
    const {customers = []} = _embedded || {};
    const [customer] = customers || [];
    const {id: customerId} = customer || {};
    return customerId === 'a';
}

z().then(log,$error);

But the transpiled code has a spurious declaration let undefined;, which is clearly a bug and might be causing an error downstream

(fixed this link, and logging errors) http://nodent.mailed.me.uk/#%2F%2F'use%20strict'%3B%0Aasync%20function%20z()%20%7B%0A%20%20%20%20await%20Promise.resolve(0)%3B%0A%20%20%20%20const%20customerSearchResults%20%3D%20%7B%0A%20%20%20%20%20%20%20%20body%3A%20%7B%0A%20%20%20%20%20%20%20%20%20%20%20%20_embedded%3A%20%7B%0A%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20customers%3A%20%5B%7B%0A%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20id%3A%20%22a%22%0A%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%7D%2C%7B%0A%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20id%3A%20%22b%22%0A%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%7D%5D%0A%20%20%20%20%20%20%20%20%20%20%20%20%7D%0A%20%20%20%20%20%20%20%20%7D%2C%0A%20%20%20%20%20%20%20%20status%3A%20%22status%22%0A%20%20%20%20%7D%3B%0A%20%20%20%20const%20%7Bbody%3A%20responseBody%2C%20status%7D%20%3D%20customerSearchResults%3B%0A%20%20%20%20const%20%7B_embedded%20%3D%20%7B%7D%7D%20%3D%20responseBody%3B%0A%20%20%20%20const%20%7Bcustomers%20%3D%20%5B%5D%7D%20%3D%20_embedded%20%7C%7C%20%7B%7D%3B%0A%20%20%20%20const%20%5Bcustomer%5D%20%3D%20customers%20%7C%7C%20%5B%5D%3B%0A%20%20%20%20const%20%7Bid%3A%20customerId%7D%20%3D%20customer%20%7C%7C%20%7B%7D%3B%0A%20%20%20%20return%20customerId%20%3D%3D%3D%20'a'%3B%0A%7D%0A%0Az().then(log%2C%24error)%3B%0A~options~%7B%22mode%22%3A%22lazyThenables%22%2C%22promiseType%22%3A%22Zousan%22%2C%22noRuntime%22%3Afalse%2C%22es6target%22%3Afalse%2C%22wrapAwait%22%3Afalse%2C%22spec%22%3Afalse%7D

djMax commented 7 years ago

(that link goes to a 192.) I think that "let undefined" is really supposed to be "let _embedded" - it's missing a decl for that right? And since there's a use strict up top, boom.

djMax commented 7 years ago

My example doesn't seem to break it like the real code, so I'll see if I can make it fail fully.

MatAtBread commented 7 years ago

Yep, you're right. I'll work out where I lost the identifier, hopefully an easier fix than your other issue in nodent

djMax commented 7 years ago

Boy I hoped the other one was easier. :) Unfortunately I think this isn't the last, because I worked around it by just not destructuring, but now there's some other problem (still diagnosing). THe background is we're trying to switch a big code base (lots of microservices) from async-to-generator to fast-async, so we're probably a pretty "wide" test case. But as long as your up for fixing 'em, we're up for finding 'em.

MatAtBread commented 7 years ago

I live for fixing.

djMax commented 7 years ago

Here we go...

const c = { body: {}, status: 200 };

async function syncfn(customerSearchResults) {
  const { body: responseBody, status } = customerSearchResults;
  const { _embedded = {} } = responseBody;
  const { customers = [] } = _embedded || {};
  const [customer] = customers || [];
  const { id: customerId } = customer || {};
  console.log('The answer is', customerId);
}

async function getResults() {
  await Promise.resolve(0);
  return c;
}

async function asyncfn() {
  const customerSearchResults = await getResults();
  const { body: responseBody, status } = customerSearchResults;
  const { _embedded = {} } = responseBody;
  const { customers = [] } = _embedded || {};
  const [customer] = customers || [];
  const { id: customerId } = customer || {};
  console.log('The answer is', customerId);
}

syncfn(c);
asyncfn(c);
djMax commented 7 years ago

Ah, and here's the other one I saw:

const c = { body: {}, status: 200 };

async function syncfn(customerSearchResults) {
  const { body: responseBody, status } = customerSearchResults;
  const { _embedded = {} } = responseBody;
  const { customers = [] } = _embedded || {};
  const [customer] = customers || [];
  const { id: customerId } = customer || {};
  console.log('The answer is', customerId);
}

async function getResults() {
  await Promise.resolve(0);
  return c;
}

async function asyncfn() {
  let customerSearchResults;
  try {
    customerSearchResults = await getResults();
    console.log('DONE');
  } catch (error) {
    console.log('EXCEPTION');
  }
  const { body: responseBody, status } = customerSearchResults;
  const { _embedded = {} } = responseBody;
  const { customers = [] } = _embedded || {};
  const [customer] = customers || [];
  const { id: customerId } = customer || {};
  console.log('The answer is', customerId);
}

syncfn(c);
asyncfn(c);

Notice that BOTH "done" and "exception" are printed even though the exception (same as before) comes from AFTER the catch handler.

djMax commented 7 years ago

which, btw, is because of this:

    try {
      return Promise.resolve(getResults()).then(function ($await_3) {
        try {
          customerSearchResults = $await_3;
          console.log('DONE');
          return $Try_1_Post.call(this);
        } catch ($boundEx) {
          return $Try_1_Catch($boundEx);
        }
      }.bind(this), $Try_1_Catch);
    } catch (error) {
      $Try_1_Catch(error)
    }

The $Try_1_Post should not be inside that larger try (I suppose instead it has to be in another try with a $Try_2_Catch that rejects the overall promise or something. Turns out this async/await thing is damn hard :)

MatAtBread commented 7 years ago

Moved the latter case to nodent (and I'll move the former too). And yes, it gets fiddly around exceptions. Nodent has been in use for around 2 years, and these haven't turned up before, so you definitely get the 2017 prize (so far)

matAtWork commented 7 years ago

Both the above issue (and another one related to the order of assignments in declarations) are fixed by https://github.com/MatAtBread/nodent/releases/tag/3.0.14, which I'll publish to npm today.

matAtWork commented 7 years ago

Upstream fixes released in https://github.com/MatAtBread/nodent/releases/tag/3.0.14