Skip to content
This repository has been archived by the owner on Feb 8, 2020. It is now read-only.

Strange parser error when an if statement is at the top of an async arrow function #61

Closed
TooTallNate opened this issue Sep 22, 2017 · 8 comments

Comments

@TooTallNate
Copy link
Contributor

Apologies if this is not a bug in parse-function itself, but I've tried this with all 3 recommended AST parsers and they all exhibit the same problem.

A parser error is thrown when an async arrow function has an if statement at the top. Ex:

$ cat t.js
const parser = require('parse-function')()

const fn = async (req, res) => {
  if (req) {}
}

console.log(parser.parse(fn));

$ node t
/test/node_modules/parse-function/node_modules/babylon/lib/index.js:812
    throw err;
    ^

SyntaxError: Unexpected token, expected { (1:19)

Strangely, the same error is thrown even if the if statement is commented out!

$ cat t.js
const parser = require('parse-function')()

const fn = async (req, res) => {
  //if (req) {}
}

console.log(parser.parse(fn));

$ node t
/test/node_modules/parse-function/node_modules/babylon/lib/index.js:812
    throw err;
    ^

SyntaxError: Unexpected token, expected { (1:19)

Now, it works if I remove the if statement:

$ cat t.js
const parser = require('parse-function')()

const fn = async (req, res) => {
}

console.log(parser.parse(fn));

$ node t
{ name: null,
  body: '\n',
  args: [ 'req', 'res' ],
  params: 'req, res' }

It also works if I change it to a regular async function instead of an arrow function:

$ cat t.js
const parser = require('parse-function')()

const fn = async function (req, res) {
  if (req) {}
}

console.log(parser.parse(fn));

$ node t
{ name: null,
  body: '\n  if (req) {}\n',
  args: [ 'req', 'res' ],
  params: 'req, res' }

So, strange behavior to say the least. Or maybe I'm doing something dumb. V8 at least has no problem parsing any of these versions. Thanks in advance for any support!

@tunnckoCore
Copy link
Owner

tunnckoCore commented Sep 22, 2017

Hey there! 👋 Thanks for the report, it sounds me like problem here.
I bet it's because of this here src/index.js#L106-L113.

The idea about that if there, is if we have passed async method, then we should wrap it in object, so the parser can parse it correctly.

Can you try to add this check too and try again if it works

const isAsyncArrow = result.value.startsWith('async')

if (!(isFunction || isAsyncFn || isAsyncArrow) && isMethod) {
  result.value = `{ ${result.value} }`
}

Another reason and probably limitation is that we use babylon.parseExpression by default here, but as i remember, it was intentional.

@TooTallNate
Copy link
Contributor Author

@charlike The patch works for me 👍 Thank you for the quick turnaround on that!

@tunnckoCore
Copy link
Owner

Sweeeet! :D I love myself sometimes for such fast responses with good end :)

PR is welcome, otherwise i can push patch in coming days. Let me know if you need some help on contributing.

@TooTallNate
Copy link
Contributor Author

Well it's your patch. I look forward to the upcoming release 👍

@the1mills
Copy link

🐯🐯🐯🐯🐯🐯🐯🐯

@tunnckoCore
Copy link
Owner

@the1mills what? 😹 haha

@TooTallNate actually, yea, but it's not a problem for me. Go pr if you need it more soon.

@TooTallNate
Copy link
Contributor Author

Please see #63.

@charlike P.S. I'd be happy to be added as a contributor, if this module has become a burden on your time.

tunnckoCore pushed a commit that referenced this issue Sep 26, 2017
* add case for async arrow functions when wrapping

Fixes #61.

* add missing `done()` to test case
@tunnckoCore
Copy link
Owner

heya, just seen your comment here.

if this module has become a burden on your time

no, it's not, just i'm in period of switching ISP providers and following through smartphone is hard, haha.

i'll add you once I get stability and time, no problem :)

ericmorand added a commit to ericmorand/parse-function that referenced this issue Feb 9, 2018
(a = (doSomething(), doSomethingElse(), true)) => {} is a prefectly valid syntax that is used
extensively by code instrumenters. When a list of expressions is used as a default value, only the
last expression is the actual default value. This commit add support for this syntax.
TAG: latest

fixes tunnckoCore#61
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants