Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Named Functions #15

Closed
jashkenas opened this issue Dec 28, 2009 · 90 comments
Closed

Named Functions #15

jashkenas opened this issue Dec 28, 2009 · 90 comments

Comments

@jashkenas
Copy link
Owner

For debugging purposes, it would be better if CoffeeScript generated only named functions, ie:

square: x => x * x.

Compiles into:

var square = function square(x) { return x * x; };

Questions:

Are there any namespace problems with this, if function names in different scopes collide? If so, what are the consequences (debuggers getting confused, perhaps)? Is it safe to take the final portion of the id as the function name when defining a function on a nested object?

@jashkenas
Copy link
Owner Author

Reference for named functions in JS:
http://yura.thinkweb2.com/named-function-expressions/

@jashkenas
Copy link
Owner Author

Since we're always going to name the function with the same name as the variable, I think we can avoid most of the pitfalls of the above article. It is going to leak memory in Internet Explorer, if you create 100000 named function objects, but that's an affordable price. This is on master now, closing the ticket.

@weepy
Copy link

weepy commented Dec 31, 2009

There's a problem with functions with the same name as variable
E.g.

x:1
y: {}
y.x: => 3

should either

  1. throw a compiler error,
  2. make the function anonymous in this instance
  3. name the something else

@jashkenas
Copy link
Owner Author

I'm not so sure that this is a problem. It seems to behave correctly when I try it. This is now a test case in test/fixtures/execution, that prints all true.

x: 1
y: {}
y.x: => 3

print(x is 1)
print(typeof(y.x) is 'function')
print(y.x() is 3)
print(y.x.name is 'x')

@jashkenas
Copy link
Owner Author

Let me know if you're hitting any errors or unexpected behavior, with a test case, and I'll reopen this ticket.

@TrevorBurnham
Copy link
Collaborator

One year later...

I'm wondering why this behavior was changed, so that square = (x) -> x * x no longer gives the resulting function the name square (which would be useful for stack traces).

Searching through later issues doesn't give me much. I can't make heads or tails of issue 758 from October; it begins "Now that we have a format for allowing named functions without breaking IE..." What was that referring to?

@TrevorBurnham
Copy link
Collaborator

Ah, just found the full explanation in the FAQ here.

@sethvargo
Copy link

I'm going to chime back in on this one...

I've read and fully understand the IE issues with named functions. However, I think it's asinine that CoffeeScript explicitly disallows the use of named functions just to support IE. By default, CoffeeScript should not allow for named functions, however, why can't we have an alternative syntax or optional compile-time flag that let's the user determine if they want named functions, not that language.

CoffeeScript has become increasingly popular among NodeJS developers - developers who don't need to support IE - they are writing server-side language. They could leverage so much more if they could name functions... I cite the example of express. If they could access the named function in a routes file, they could "guess" the name of the view (like rails) using arguments.callee.caller.name... but that's impossible when everything is an anonymous function when compiled.

Bottom line - I understand why it was taken out. It needs to be put back in as a non-default compile option or alternative syntax. You've taken away one of the best features of Javascript :'(

@em
Copy link

em commented Jan 5, 2012

I really need this. I didn't realize coffeescript with OK with removing javascript features. I thought it was supposed to be nothing fancy, and I think attempting to remove the responsibility of cross-platform programming is pretty much as fancy as you could get.

@sethvargo
Copy link

When was this fixed? Can someone cite a commit?

@michaelficarra
Copy link
Collaborator

@sethvargo: it hasn't been fixed and never will. There's no need for NFEs or FDs. Your example above is not convincing. arguments.callee is a poison pill in strict mode, and highly highly discouraged outside of strict mode. Like eval, it doesn't allow the interpreter to do almost all of its optimisations. If you really care, add a displayName property to your functions after defining them.

@em
Copy link

em commented Jan 5, 2012

Well, aside from debugging and profiling where function names are essential. I use the .name property that v8 puts on all functions for a kind of monkey patching. The problem is not that I can't design an API which uses strings and a hashmap (object), but aside from it looking ugly, the entire codebase at hand is written in js which has named functions, so I now need two alternate interface for patching methods one using strings so coffeescript can work, and another using function names, because coffeescript is not compatible with javascript. Alternatively I can't use coffeescript in the app.

You are making the assumption that a library will never use function.name, but if anything does (like my lib) then it cannot be used by coffeescript, unless it repurposes itself for coffeescript's sake. : \

e.g.

var module = {
foo: function() { /.../ }
, bar: function() { /... }
};

function patch(functionToPatch) {
module[functionToPatch.name] = functionToPatch;
}

patch(function foo() {
// my replacement foo
});

Again, "patch" could be repurposed for coffeescript, the problem is that coffeescript probably shouldn't be requiring js to change for its sake.

Also... the real problem creating the IE issue is the assumption that "a = ->" generates "function a()", instead of an additional syntax for named functions. The idea that a named function is the same as a named variable holding a function, is a false premise of how javascript works.

@michaelficarra
Copy link
Collaborator

@em: name is a non-standard property. That's what you get for relying on it being set by your interpreter. You should be assigning an identifying value of your own. Are you also going to rely on iteration order of keys in a for-in loop (another assumption people often make that's not guaranteed, in case you didn't know) and then complain about coffeescript?

edit: regarding debugging, most debuggers actually look at the displayName property, which you should be setting yourself.

@em
Copy link

em commented Jan 5, 2012

The IE bug is not standard either, so what's your point? The .name property is one example of how different environments can do different things for named-functions and anonymous-functions. They're different things. The other example bodes the same, debugging and profiling use function.name to build call stacks.

Honestly, I bet if you asked Brendan Eich why he gave javascript named functions at all and didn't make everything anonymous he'd cite the same point. Sheesh.

@jashkenas
Copy link
Owner Author

Let's be pleasant, folks.

@ghost
Copy link

ghost commented Jan 5, 2012

IIRC, var square = function square(){ ... }; or square = function square() { ... }; should work...the leakage in IE occurs when the reference and the function name are different (var a = function b() { ... }). The square variable declaration is still overwritten with the square function declaration, but, because they are identical, the former should be garbage collected immediately.

Pinging @jdalton; he clarified this point for me some time ago.

@michaelficarra
Copy link
Collaborator

@em:

named-functions and anonymous-functions. They're different things.

They're really not different things. NFEs and anonymous FEs are identical except NFEs add their name to their body's scope. Other than that, they're supposed to behave exactly the same. FDs are different because they respect odd hoisting rules and they add their names to the containing scope.

The IE bug is not standard either, so what's your point?

That's just a reason for not using the extraneous feature that we're not providing for you. We're not providing it because it's not necessary and, as the IE bug points out, can even be harmful. We also don't provide with statements or unqualified access to the undefined global. Sure, some interpreters have errors related to those features, but those are only reasons not to use the features. We don't include the features because they're just not useful.

debugging and profiling use function.name to build call stack

As I said above, they use displayName first. You should be defining that value if you want better debugging support.

note: in case you aren't familiar with the common acronyms, NFE = named function expression, FE = function expression, FD = function declaration

@michaelficarra
Copy link
Collaborator

@kitcambridge: you are correct. The IE bug treats NFEs similarly to FDs in that it adds the name to the containing scope instead of just the function body's scope.

@jdalton
Copy link

jdalton commented Jan 5, 2012

I was summoned by @kitcambridge to confirm that var square = function square(){ ... }; takes care of the IE memory consumption problem. I used this in my code/projects to handle it.

@sethvargo
Copy link

Obviously there's some hostility here. Frankly I think it's ridiculous. I don't understand why you won't add the functionality back in. Since there's such hostility, it should be an option, not a default. However, you are violating this very description of coffeescript (via CoffeeScript.org):

The golden rule of CoffeeScript is: "It's just JavaScript". The code compiles one-to-one into the equivalent JS, and there is no interpretation at runtime. You can use any existing JavaScript library seamlessly from CoffeeScript (and vice-versa).

Well, it's not "just JavaScript". There's something I can do in plain JS (NFE) that I can't do with coffeescript because its creators are too strongly opinionated against it. Either implement it as a non-standard option, or change the description on coffeescript.org.

@sethvargo
Copy link

Furthermore, labeling this as "fixed" was quite misleading... I suggested "blatantly ignored due to overzealous opinions"...

@jashkenas
Copy link
Owner Author

@sethvargo At the time the issue was closed, named functions existed on master. They were later removed for the reasons detailed above.

@sethvargo
Copy link

Well, someone should remove the label... Without reading the entire thread, it's very easy to think this issue has been resolved...

@michaelficarra
Copy link
Collaborator

@sethvargo: CoffeeScript's golden rule doesn't apply to constructs, but functionality. As I said above, there's no way to put an unqualified reference to the undefined global or a with statement (without the god-awful backticks) in your output JS. But that doesn't prevent you from doing anything. It's still a one-to-one (injective) compilation, but it's not onto (surjective). I think you're confusing the injective relationship with a bijective relationship. Being injective but no surjective is not a problem in our case. The functionality of the JS constructs that don't have CS equivalents can be accomplished through combinations of CS constructs.

Also, I'm not sure what hostility people are talking about. I don't see any. If it was directed at me, I didn't mean to come off as hostile.

@em
Copy link

em commented Jan 6, 2012

Yeah, I don't see any hostility either. My "Sheesh"? Sorry, it resonated as light-hearted in my head.

@rlidwka
Copy link

rlidwka commented Feb 2, 2012

@michaelficarra

If you really care, add a displayName property to your functions after defining them.

Do you really think that manually adding displayName property to all functions is a very good idea?

Really, I'm working with server-side javascript, so, I cant imagine any reason why some old microsoft's garbage is poisoning it for me now... :)

@rlidwka
Copy link

rlidwka commented Feb 2, 2012

Well... fortunately, it is one-line fix. I've just replaced if (this.ctor) condition with if (this.name !== undefined) in nodes.js:

code = 'function';
/*if (this.ctor)*/ if (this.name !== undefined) code += ' ' + this.name;
code += '(' + vars.join(', ') + ') {';

and now it works just fine.

@em
Copy link

em commented Feb 2, 2012

Chiming in to say I tested every major browser, safari, ff, and chrome. None of them use 'name' or 'displayName' for profiling or console display of function names.

@rlidwka
Copy link

rlidwka commented Feb 2, 2012

@em:
Node.js and Chrome are using Function.name to build stack trace (see here and here).

Sometimes, it will print my function name instead of unknown source saving me some seconds of searching... reason enough for me.

For production enviroment with some buggy browsers we can always turn off this feature, of course.

@jashkenas
Copy link
Owner Author

jashkenas commented Dec 23, 2016

@GeoffreyBooth @lydell @vendethiel

Now that IE8 and below aren't relevant anymore, is there a good reason why CoffeeScript 2.0 shouldn't bring back our old-school named-by-default functions everywhere?

edit: To be clear — I'm not talking about function declarations. I'm talking about all functions being named in addition to assigned, like we used to have, briefly.

@connec
Copy link
Collaborator

connec commented Dec 23, 2016

I'd be concerned about users expecting the same behaviour from bound functions, for which there is no named syntax.

With the rules of the ES spec named functions are basically redundant in CS (where an assignment is needed for the name anyway).

@jashkenas
Copy link
Owner Author

jashkenas commented Dec 23, 2016

I'd be concerned about users expecting the same behaviour from bound functions, for which there is no named syntax.

Huh? That's not true. We can put names on bound functions in exactly the same way as unbound ones:

hello = ->
hello = function hello() {};

hello = =>
hello = (function(_this) {
  return function hello() {};
})(this);

@connec
Copy link
Collaborator

connec commented Dec 23, 2016

Fair point. I was thinking of the 2 branch where bound functions are now compiled to ES6 arrow functions

hello = =>
hello = () => {};

@jashkenas
Copy link
Owner Author

Oh, of course -- sorry. I forgot about those.

@avalanche1
Copy link

avalanche1 commented Dec 24, 2016

@jashkenas what about function hoisting? let's also support that!

@penne12 thanks for ``function test(){; do -> - didn't know about the `do ->` part 👍

@GeoffreyBooth
Copy link
Collaborator

@jashkenas We were discussing named classes here in the CS2 reimplementation of classes. Basically the CS2 version of classes will output code like var Foo; Foo = class Foo {. I was suggesting that if we’re doing this, we should be consistent and do the same for functions.

So yes, I think function declarations like var foo; foo = function foo () { would be great. Even if we can’t do it in all circumstances, we might as well do it where we can to make debugging easier. I’m not worried about the increase in file size, because CS output wasn’t meant to be minified in the first place, and a minifier would make short work of this duplication.

alangpierce pushed a commit to alangpierce/coffeescript that referenced this issue Jan 2, 2017
* fix: add missing `Base#locationData` property

* fix: `Literal#value` is a string, not another node

* fix: add missing `Block#expressions` property

* fix: `Parens#body` is a block
@mauskin
Copy link

mauskin commented Jan 9, 2017

what about function hoisting?

@avalanche1 It was clearly pointed out that function declaration (or hoisting) is not a part of this thread. Please create a separate issue where we could have a focused discussion about its necessity and implementation.

@avalanche1
Copy link

avalanche1 commented Jan 9, 2017

well, function hoisting without named functions doesnt make sense to me - so I'll wait if named functions are set upon as an agreed solution - then I'll make a sep. request if needs be.

@avalanche1
Copy link

avalanche1 commented Jan 26, 2017

Void

WHOA! Super duper kool!
3a.m. and I've just found that named functions DO EXIST in Coffeescript. (Well, sort of named.. 😃 )
And they ARE hoisted to!! (When you declare in the following way:)

foo()
foo = f = ->log arguments.callee.name
// foo

The following works for objects:

obj=
  foo: foo = ->log arguments.callee.name

Finally we can drop all the backtick voodoo and other magic. Hurray! 😺

I think this can be closed.
But I think the Functions section of the docs should be amended.

@benaubin
Copy link

@avalanche1 That's not coffee script - it's the browser doing it for you, and it has limited support (and is undocumented behavior). This issue still should be worked on to make it work in all browsers.

@avalanche1
Copy link

avalanche1 commented Jan 26, 2017

@penne12 , huh?

  1. Don't spoil all the happiness!
  2. Care to explain?

R u talking about argumetns.callee? Well, it doesnt matter if it's being deprecated - functions are hoisted and can be called before being declared - KOOL!
Pllus, I need arguments.callee.name exclusively for debugging & testing purposes.

@benaubin
Copy link

benaubin commented Jan 26, 2017

Oops... sorry bout that 😨

Anyways, basically, a named function looks like this:

function namedFunction(){
  // this is a namedFunction
}

But, with your example, CoffeeScript generates:

var f, foo;

foo();

foo = f = function() {
  return log(arguments.callee.name);
};

Which means that the function isn't named - but some browsers will assume the name based on what they're assigned too, but not always.

In terms of hoisting, CoffeeScript is smart and declares variables the first time they're used, and many browsers will do a "first past" while parsing Javascript to define variables before running the rest of the code (as far as I understand it, maybe someone more familiar with the inner workings of the language could explain better).

@avalanche1
Copy link

avalanche1 commented Jan 27, 2017

I see.
Does this feature have a name? I'd like to lookup JS engine compatibility.

@GeoffreyBooth
Copy link
Collaborator

@avalanche1 It’s not a feature with a name like you would find in an ES6 compatibility table. I would refer to it as debugging tools inferring anonymous function names from their assigned references. See https://www.bennadel.com/blog/2836-anonymous-functions-assigned-to-references-show-up-well-in-javascript-stack-traces.htm

@avalanche1
Copy link

avalanche1 commented Jan 27, 2017

Ahem, after further investigating, it turns out that what I've posted above is actually void.
I was testing aforementionned techniques in Coffeescript Console extension for Chrome;
On the first run it throws en error, on the second, though - it works as described above.
Guess, that's the effect of working till 3AM.

Anyway:
still no function hoisting;
function name inferred from variable assignement works only if JS engine supports it.

@benaubin
Copy link

benaubin commented Jan 27, 2017

Here's a simple little script if you want to force names onto functions, but this uses runtime evaluation, and might not be the best choice except in some very specific scenarios.

DO NOT LET THE USER PICK THE FUNCTION'S NAME, OR THEY WILL BE ABLE TO EXECUTE ARBITRARY JAVASCRIPT CODE

nameFunc = (n, func) ->
  new Function("func",
    "return function #{n}(){return func.invoke(this, Array.prototype.slice.call(arguments))}"
  )(func)

Then just:

nameFunc 'namedFunc', (a) -> console.log a

might be somewhat broken, also shouldn't be used for user input.

@avalanche1
Copy link

@penne12 , thanks mate!
Thankfully my dev environment supports named functions. And I'll make sure not to use them in production builds.

@GeoffreyBooth
Copy link
Collaborator

Per #4531, it was decided that CoffeeScript will not be supporting function declarations (outputting function foo() as opposed to foo = function ()) or outputting named functions as part of the currently output function expressions (so still foo = function(), not foo = function foo()).

@dotnetCarpenter
Copy link

dotnetCarpenter commented May 2, 2017

@GeoffreyBooth I was cleaning up an old coffeescript project and came across arguments.callee, because coffeescript does not have support for named functions.

if obj instanceof baseStream && !obj._readableState.ended
  obj.on('end', next)
  obj.on('data', ->
    obj.removeListener('end', next)
    obj.removeListener('data', arguments.callee)
    next()
  )
else
  next()

In es5 you would do:

if (obj instanceof baseStream && !obj._readableState.ended) {
  obj.on('end', next)
  obj.on('data', function data() {
    obj.removeListener('end', next)
    obj.removeListener('data', data)
    next()
  })
} else
  next()

If coffeescript had support for named functions, you could do:

if obj instanceof baseStream && !obj._readableState.ended
  obj.on('end', next)
  obj.on('data', \data ->
    obj.removeListener('end', next)
    obj.removeListener('data', data)
    next()
  )
else
  next()

Which is a nicer than:

if obj instanceof baseStream && !obj._readableState.ended

  # don't inline event handlers in coffeescript (no support for named functions)
  # arguments.callee is deprecated so don't use it to remove event handler
  data = ->
    obj.removeListener('end', next)
    obj.removeListener('data', data)
    next()

  obj.on('end', next)
  obj.on('data', data)
else
  next()

The \name is the anti Lambda Abstractions hehe ;). Joke aside, I don't know how the syntax should be, but \ is an anonymous function in Haskell, so perhaps it could be a named function in coffee.

@connec
Copy link
Collaborator

connec commented May 2, 2017

What's wrong with

if obj instanceof baseStream && !obj._readableState.ended
  obj.on 'end', next
  obj.on 'data', data = ->
    obj.removeListener('end', next)
    obj.removeListener('data', data)
    next()
else
  next()

GeoffreyBooth pushed a commit to GeoffreyBooth/coffeescript that referenced this issue Sep 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests