Function.prototype.toString()

[xorgy]

It seems unnecessary to me that Function.prototype.toString() returns a useless placeholder token (like it does with native functions). For native functions it is fair enough, and the ECMAScript standard reflects this; but the code in this case is JavaScript, even if it's in AST form, so it seems like it would be better to more closely follow the behaviour of real JavaScript. Furthermore, there is at least some code which relies on Function.prototype.toString(), whether you feel that's a prudent idea or not.

How much trouble would it be to have Function.prototype.toString() lazily emit a valid (or standard) JavaScript source representation of the binary AST instead?

[myshov]

Yes I agree with @xorgy. There was usage of Function.prototype.toString() at least in the first version of Angular for implementation of dependency injection.

[ExE-Boss]

You could always display the arguments, eg.: example toString() result:

function example(foo, bar) {
	[sourceless code]
}

That would avoid breaking the old Angular dependency injection.

[xorgy]

@ExE-Boss That's an interesting middle ground, though I'm not sure it's quite right either, makes it necessary to use a regex to tell that the code isn't real, though I guess not all that many people are going to be accidentally evaling some garbage they got from Function.prototype.toString().

[ExE-Boss]

makes it necessary to use a regex to tell that the code isn't real

You already need to do that with native functions, where the result is slightly different between browsers: tc39/Function-prototype-toString-revision#21.

Also, the ESNext native function toString() method will optionally be returning function parameters.

[vdjeric]

It seems unnecessary to me that Function.prototype.toString() returns a useless placeholder token [...] there is at least some code which relies on Function.prototype.toString()

You are right and we want toString() in BinAST to work the same way as it does for plain JS but it's just a matter of implementation priorities. Right now we're working on a new file format for BinAST and decoder performance so we've shelved toString() for the timebeing.

[xorgy]

@vdjeric Fair enough, though I suspect regardless of what BinAST looks like, it is not likely to be difficult for implementers to write some sort of toString, especially the sort that @ExE-Boss is describing (since there's not much that can be done to collapse arguments, especially with object destructuring, so presumably names will be intact).

As long as a slightly more useful Function.prototype.toString is not off the table, I'm happy. :- )

[fabiosantoscode]

One of the things that make JavaScript great is the fact you can eval() most functions' string representations (plus a pair of parentheses) and get a function back. This is not recommended, but is still great for a lot of reasons:

• You can inline a function into a data: URL (which is useful, for say, creating a Worker on-demand)
• You can read arguments and comments as said above and perform actions based upon the results

And probably more things I can't remember right now.

IMO the VM should generate a string representation from its internal AST, on-demand, when the toString method is called.

[rossberg]

@fabiosantoscode:

• You can inline a function into a data: URL

That's a bug, not a feature. Unless you want to help attackers.

[fabiosantoscode]

@rossberg not like a security issue, more like this:

const worker = new Worker('data:text/javascript,(' + () => {
  /* worker code goes here... */
} + ')()')

In this example, it helps you define a worker next to the place it's going to be used and not in a new file. There's probably more interesting things you can do with data-urls and toString. I can't see how this is a bug.

[fabiosantoscode]

Also, I can't overstate how many times I use Function.prototype.toString to read the source code of some unknown callback in the browser console or the node console.

[guybedford]

Workers do often need data / blob URL initialization tricks in many tooling approaches Note that module workers are not cross origin by default (although their dependencies can be) so in many cases this data URL trick is absolutely needed in order to support loading module workers from CDN URLs..

…

On Mon, 24 Jun 2019 at 21:06, Fábio Santos ***@***.***> wrote: @rossberg <https://github.com/rossberg> not like a security issue, more like this: const worker = new Worker('data:text/javascript,(' + () => { /* worker code goes here... */ } + ')()') In this example, it helps you define a worker next to the place it's going to be used and not in a new file. There's probably more interesting things you can do with data-urls and toString. I can't see how this is a bug. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#76?email_source=notifications&email_token=AAESFSRCCFFHPF5WFMVG653P4ELMRA5CNFSM4HNYH642YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYN5BTY#issuecomment-505139407>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAESFSUINVGW5AWAROJNCNLP4ELMRANCNFSM4HNYH64Q> .

[C-Ezra-M]

If there is need to have Function.prototype.toString() compatible with eval(), it would be best for comments to not be saved in the AST, because they are not executed.

Python has AST-relevant public APIs built into the standard library as the ast module, and it's evident that the Python AST parser doesn't save comments (Python uses # to denote them):

import ast

print(ast.unparse(ast.parse("""def a():
    # does stuff
    return 0""")))

which outputs:

def a():
    return 0

The comment isn't there because it's not necessary for the function to work. Docstrings are preserved, though.