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

Giving name to anonymous functions #23

Closed
jridgewell opened this issue Apr 3, 2020 · 8 comments · Fixed by #24
Closed

Giving name to anonymous functions #23

jridgewell opened this issue Apr 3, 2020 · 8 comments · Fixed by #24

Comments

@jridgewell
Copy link
Member

jridgewell commented Apr 3, 2020

Context: babel/babel#11362

As currently spec'd, Logical Assignment does not assign a name to anonymous functions. Consider:

let foo;
foo ??= function() {};
console.log(foo.name);

This currently should log undefined. But I think it'd be totally reasonable for this to log foo instead.

Babel hits this issue with its transform:

// Transformed Output
let foo;
foo ?? (foo = function() {});
console.log(foo.name);

That (foo = function() {}) will assign foo as the name for the anonymous function. We can work around this by using an IIFE to indirect. But I don't think the name hurts here.

@michaelficarra
Copy link
Member

michaelficarra commented Apr 3, 2020

This only makes sense if you think of the rewrite of a ??= b as a ?? (a = b) instead of a = a ?? b. I think most people will think of it as the latter. My understanding is that they're equivalent (for any a that participates in FNI, ignoring with). Because of this, I think we could have a valid argument both for and against FNI.

@hemanth
Copy link
Member

hemanth commented Apr 3, 2020

Makes absolute sense to have function name here, but as @shapesecurity mentioned most of us would interpret a ??= b as a = a ?? b unless otherwise mentioned or people understand that this behavior is for anonymous functions.

@ljharb
Copy link
Member

ljharb commented Apr 3, 2020

ftr you don't need an IIFE to work around it, you can do (0,function () {})

@dcrousso
Copy link
Member

dcrousso commented Apr 3, 2020

I could go either way on this, but I have a slight preference for it not using the name.

In the same way that

let x = false || function() { };
let y = true && function() { };
let z = null ?? function() { };

don't assign the name, I don't think this should either. I feel like the only time we should use the variable name for the function name is if it's completely straightforward (e.g. let foo = function() { };).

@michaelficarra
Copy link
Member

Ugh I guess we really shouldn't ignore the existence of with...

Given this observable equivalence for get/set invocations:

let a = true;
with ({
  get a() { console.log('get a'); return a; },
  set a(x) { console.log('set a'); a = x; },
}) {
  // this line
  a ||= function(){};
  // is observably equivalent to this line:
  a || (a = function(){});
  // not equivalent to this line (the setter is invoked):
  a = a || function(){};
}

then I'd expect their handling of FNI should be the same.

let a = false, b = false;
with ({
  get a() { console.log('get a'); return a; },
  set a(x) { console.log('set a'); a = x; },
  get b() { console.log('get b'); return b; },
  set b(x) { console.log('set b'); b = x; },
}) {
  a ||= function(){};
  b || (b = function(){});
}
assert.eq(a.name, 'a');
assert.eq(b.name, 'b');

@anba
Copy link

anba commented Apr 3, 2020

I don't see why the same sequence of get/set invocations should have any impact whether or not the function gets an inferred name. a || ((a) = function(){}); triggers the same proxy traps, but doesn't assign a function name.

@michaelficarra
Copy link
Member

@anba My thinking is that users will think of the syntax introduced in this proposal as simple sugar for one of a = a || b or a || (a = b) (for simple LHSs). When choosing between which of those two they might think of, we would want to take into account the semantic parity. Taking that into account, the most similar desugaring is a || (a = b), so we should match that with FNI as well.

@devsnek
Copy link
Member

devsnek commented Apr 20, 2020

if the left side is an identifier and the right side is an anonymous function, the function should have the name, regardless of what the operator is.

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants