-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[CS2] Named functions #4531
[CS2] Named functions #4531
Conversation
…ed, or dynamic); update tests accordingly
Is there a reason we don't just compile it to
vs
|
To note — this is the same way that CoffeeScript briefly compiled functions (probably in 2010). We removed it because it broke some versions of Internet Explorer at the time: the function declaration would clobber variables in places that it shouldn't have. I'd assume that versions of IE currently in use don't have that problem anymore. |
@vendethiel Compiling to function declarations (your suggestion) as opposed to function expressions (current behavior) changes the scope of the functions. It's also not true to the input. How would we compile |
Oh, I completly forgot |
With this PR, the following would cause an infinite loop, wouldn’t it? foo = -> console.log('foo')
obj =
foo: -> foo()
obj.foo() Babel renames the The ES2015 specification made the CoffeeScript 2 emits ES2015 code, so I don’t think it makes sense to do this in CoffeeScript. |
Which part, the function renaming or this PR in general? |
This PR in general. |
Yeah, it seems like for modern JS, we really don't need to add this:
|
Well based on the recent comments in #15, “modern JS” is only V8—Node and Chrome. Apparently debugging in Firefox and other runtimes is still aided by having the names. That said, presumably they’ll catch up, so maybe it’s not worth introducing a breaking change (however minor) for this. I don’t feel strongly one way or the other. At least we’ll be able to definitively close that issue and settle the question about whether we will ever support named functions. @jashkenas and others, do you agree with @lydell? Or is there some reason to pursue this? |
I agree with @lydell. Named functions have always been kind of hacky — subject to cross-browser incompatibilities, etc. With better support for regular variables in debugging, I don't think we need to do this. |
Okay, so if we’re not going to add this, then there’s no reason to keep #15 open. It’s settled. Can someone write a few sentences explaining why CoffeeScript deliberately lacks named functions? So I can add it to the docs near where we explain why we don’t support |
The MDN article is probably a good reference point - the summary would be that: CoffeeScript relies on the function naming capabilities of the runtime. ES2015 includes rules for inferring function names from assignments. If you wish to give a name to a function in an expression, simply assign it and the name will be inferred by the runtime. logName = (f) -> console.log f.name
logName -> # ''
logName foo = -> # 'foo'
logName foo.bind this # 'bound foo'
logName { method: -> }.method # 'method' Bear in mind that function names may change if using a name-mangling minifier. |
@connec That’s a good explanation for why users don’t need named functions, but it doesn’t answer the question of why we made the design decision to not support them. What would be the answer to the reasonable question “Why can’t I get CoffeeScript to somehow output I suppose the answer could be “because you don’t need them,” but that’s not very satisfying. Another answer could be that CoffeeScript simply doesn’t have declarations of variables, and ultimately |
You don't need to explain why CoffeeScript doesn't have them. It's on those that do want them to explain why. You just need to state that they're redundant. Why wouldn't that be satisfying? It's the position CoffeeScript takes. They're not dangerous; they're just pointless, at least in Ashkenas' opinion, and no one has proven otherwise so far. |
@carlsmith It’s for this section in the docs: http://coffeescript.org/v2/#unsupported. I think it’s nice to explain our design decisions. |
Sorry. It seemed like you were retrofitting a rationale without really meaning to. I would just say that named functions were implemented in an early version of CoffeeScript, as the feature was often requested, but it created problems in IE, so was quickly removed again. It has been asked for many times since, but to date, no one has provided an actual use case. There are benefits to functions being referred to by name in stack traces, but browsers already do a good job of identifying anonymous functions when they are assigned a name. Until a compelling case for named functions is provided, they will remain unsupported, no matter how many times they are asked for. |
Archived this branch at https://github.com/GeoffreyBooth/coffeescript-archived-branches/tree/named-functions |
I'm gonna chime in with the same example as I did in #15. I am/was cleaning up an old coffeescript project and came across 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 |
As mentioned above:
Your lambda example can be rewritten as 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() |
@connec You're right. Sorry for my coffee ignorance. Perhaps you can also look through the following. I wrote it before I saw your answer and perhaps the argument is mute. Another reason to only have named functions, is that it makes for a nicely readable program. Example (es2015): const world = getWorld()
let action = compose(x => x.doAction(), filter(compose(is, dot('property'))))
let action2 = compose(x => x.doAction(), filter(compose(is, dot('another'))))
if(condition) action(world)
else action2(world)
function curry(f) {}
function compose(...fs) {}
function filter(predicate, data) {}
function is(a) {}
function dot(property, obj) {} above: all functions with more than 1 argument are curried
Since function definition are parsed (need a better word) first in a js program, this structure is possible. With function expression, this is not the case. Not even sure how to express that in coffee. But if all functions are function definition then anonymous function could be |
That's a legitimate point @dotnetCarpenter - CoffeeScript drops supports for ES' function hoisting in favour of having a single way of defining any variable/function (via regular assignment). Whilst this does come up occasionally, I generally find that most of the code I write (in CoffeeScript and in general) is 'lazy' in that the main work in a script is exported as a function making this a non-issue. To paraphrase your example: exports.run = ->
world = getWorld()
action = compose ((x) -> x.doAction()), filter compose _is, dot 'property'
action2 = compose ((x) -> x.doAction()), filter compose _is, dot 'another'
if condition
action world
else
action2 world
curry = (f) -> ...
compose = (fs...) ->
filter = (predicate, data) -> ...
_is = (a) -> ...
dot = (property, obj) -> ... In other cases, such functions are anyway imported from a separate module: { curry, compose, filter, is: _is, dot } = require './helpers'
world = ...
action = ...
... Obviously that's a bit of a cop-out answer, and the honest one as above is that the hoisting behaviour has been sacrificed in the name of a unified way of defining variables and functions. |
I just realized that the AirBnB style guide (
https://github.com/airbnb/javascript#functions) explicitly uses named
functions.
This styleguide is heavily influential and their linting rules will
definitely be copied.
…On May 2, 2017 10:31 AM, "Chris Connelly" ***@***.***> wrote:
That's a legitimate point @dotnetCarpenter
<https://github.com/dotnetCarpenter> - CoffeeScript drops supports for
ES' function hoisting in favour of having a single way of defining any
variable/function (via regular assignment).
Whilst this does come up occasionally, I generally find that most of the
code I write (in CoffeeScript and in general) is 'lazy' in that the main
work in a script is exported as a function making this a non-issue. To
paraphrase your example:
exports.run = ->
world = getWorld()
action = compose ((x) -> x.doAction()), filter compose _is, dot 'property'
action2 = compose ((x) -> x.doAction()), filter compose _is, dot 'another'
if condition
action world
else
action2 world
curry = (f) -> ...compose = (fs...) ->filter = (predicate, data) -> ..._is = (a) -> ...dot = (property, obj) -> ...
In other cases, such functions are anyway imported from a separate module:
{ curry, compose, filter, is: _is, dot } = require './helpers'
world = ...
action = ...
...
Obviously that's a bit of a cop-out answer, and the honest one as above is
that the hoisting behaviour has been sacrificed in the name of a unified
way of defining variables and functions.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#4531 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABy6x4sKSSGKFfk9dIvF8vF3JvYyNpTVks5r12hsgaJpZM4NJ4Of>
.
|
Closes #15, the oldest open issue (from 2009!). With this PR, this:
now compiles to this:
Note the
function square(x)
, instead of justfunction(x)
. This mimics how Babel converts methods into ES5 functions.The name is not output for bound functions, which per ES spec must be anonymous; or for any function name that contains a period (
helpers.eq = ->
), bracket (method[name] = ->
) or quotes ('not': ->
). Those cases just stay anonymous like they are now.The only breaking change caused by this is when a function returns a reference to itself, as illustrated by the test that needed updating:
Since the first line now compiles to
changeMe = function changeMe() {
, the innerreturn changeMe = 2
no longer overrides the variable namechangeMe
one level up. Hopefully this is a rare enough edge case that it shouldn’t come up too often?And there’s the question of whether this is doing at all at this point, now that debuggers can often infer function names from the names of the variables they’re assigned to. My instinct is that it is still worth doing, since it does add some clarity to the output (presumably that’s by Babel does it) and there are probably some debugging tools that are helped by this.