Node 10.0.0 Function 's toString method behaves differently than in previous versions

[sadasant]

Node 10.0.0 yields an unexpected result when functions are converted to strings through the toString method. Here's what happens:

• 10.0.0: (function () { }).toString() results in: 'function () { }'.
• 9.5.0: (function(){ }).toString() results in: 'function (){ }'.

The affected line is: https://github.com/nodegit/promisify-node/blob/master/utils/args.js#L9
A possible solution is to change the RegExp to: /function.*?\(([^)]*)\)/ (for example).

[awwright]

While V8 should be conservative in what it emits, I would point out this module should also be liberal in what it accepts. Both return values are indeed valid ECMAScript, and the module here should be prepared to accept any valid function, even input not directly from Function#toString.

Perhaps a full ECMAScript parser would be overkill (note that constructions such as function/**/name/**/(a)/**/{} are valid, since comments are considered whitespace), but using \s* would certainly be better.

I would propose .match(/function\s*([^(]*)\s*\(([^)]*)\)/)[2]

[jmls]

is there any expection of when this is due to be fixed ?

[sadasant]

Well, nothing is broken if we don't upgrade to Node 10, but eventually we will want to update. It's really up to the availability of whoever can push here 🌞

[getify]

Many of us are on node10 now, and this is a big blocker to be incompatible. Please patch this regex quickly.

[lenovouser]

This is still an issue. @awwright do you want to submit a PR with your proposed regex?

[awwright]

I actually know very little about node-git, I'm here because I've stumbled across this before.

I don't know if the regexp would even work.

Also it appears V8 will, in fact, embed comments inside Function#toString() e.g.

> foo.toString().match(/function\s*([^(]*)\s*\(([^)]*)\)/)[2]
'a, b,c, /*c*/c'

(note there's no space in b,c, which the function also expects to split arguments)

[jsilvermist]

@tbranyen Any update on this issue? It's causing some pretty widespread issues.

[tbranyen]

I can check into it today, it was always a hack, but I'm a little surprised this changed. Since it's a V8 change and could break parts of the existing web.

[tbranyen]

Fixed via #02fc47c

[jsilvermist]

@tbranyen Thanks!

[sadasant]

@tbranyen thank you!