SetFunctionName - problems and inconsistencies

[Jamesernator]

So currently method, method/getter/setter methods call SetFunctionName to set the name of those methods/getters/setters (though this currently broken it's presumed this will fixed).

However setting this unconditionally has some problems, namely:

• This will throw an error if called on a function where "name" is not configurable (e.g. a frozen function)
• Functions that are returned from a (method/setter/getter) decorator multiple times will keep having their names changed
• No way to customize function names with decorators at all

Further setting names isn't even consistent, the spec does it for methods, getters and setters but doesn't for auto-accessors or classes:

function decorateClass() {
    return class DecoratedClass {}
}

function decorateMethodOrGetterOrSetter() {
    return function decoratedFunction() {
       // Just a dummy function
    };
}

function decorateAccessor() {
   return {
       get: function decoratedAccessorGetter() {

       },
       set: function decoratedAccessorSetter() {

       },
   };
}

@decorateClass
class SomeClass {
    @decorateMethodOrGetterOrSetter
    someMethod() {

    }

    @decorateMethodOrGetterOrSetter
    get someGetter() {
       return "dummy";
    }

    @decorateMethodOrGetterOrSetter
    set someSetter(value) {

    }

    @decorateAccessor
    accessor someAccessor;
}

// CHANGED:
console.log(Class.prototype.someMethod.name); // someMethod
console.log(Object.getOwnPropertyDescriptor(Class.prototype, "someGetter").get.name); // someGetter
console.log(Object.getOwnPropertyDescriptor(Class.prototype, "someSetter").get.name); // someSetter

// NOT CHANGED:
console.log(SomeClass.name); // DecoratedClass
console.log(Object.getOwnPropertyDescriptor(Class.prototype, "someAccesssor").get.name); // decoratedAccessorGetter
console.log(Object.getOwnPropertyDescriptor(Class.prototype, "someAccesssor").set.name); // decoratedAccessorSetter

---

In most regards I think setting all these names is probably the best default, though it should really fail gracefully if the function "name" is not configurable (e.g. frozen).

Though it would be nice to have an API to change the name if possible:

function named(name) {
    return function decorator(_, ctx) {
        // Only available for function-like contexts, i.e. method/getter/setter/auto-accessor/
        ctx.setFunctionName(name);
    }
}

@named("AST.AdditionNode")
class ClassNode {

}

export const AST = { ClassNode, ...etc };

[ljharb]

It's more than that; SetFunctionName can't actually ever be called on a function unless the name property has been entirely deleted, not just made configurable.

[Jamesernator]

It's more than that; SetFunctionName can't actually ever be called on a function unless the name property has been entirely deleted, not just made configurable.

Well I was presuming that the suggested fix were to be added. Though actually even with that fix the usage is still broken, in particular SetFunctionName also sets F.[[InitialName]] which seems like a completely incorrect thing to do and would expose a completely new capability.

i.e. At present:

Object.defineProperty(parseInt, "name", { configurable: true, value: "NOT_PARSE_INT" });

console.log(parseInt.toString());

will always print:

function parseInt() { [native code] }

regardless of what you change "name" to.

However if the aforementioned proposed "fix" is done, then actually this suddenly becomes possible:

function decorate() {
    return parseInt;
}

class SomeClass {
   @decorate
   NOT_PARSE_INT() {

   }
}

// Now prints "function NOT_PARSE_INT() { [native function] }"
console.log(parseInt.toString());

[Jamesernator]

In most regards I think setting all these names is probably the best default

Thinking about this more with regards to stack traces I'm actually thinking it might be an outright bad idea. For example if a decorated method were to have it's name automatically set then an error thrown in the original function makes the same name appear for otherwise distinct functions.

e.g. If we do:

function decorate(method) {
    return function decoratedMethod(...args) {
        // ... such and such
        return method.call(this, ...args);
    }
}

class Foo {
    @decorate
    method() {
        if (someCondition()) {
            throw new Error(`some error`);
        }
        // ...
    }
}

then stack traces will appear like:

Error: some error
    at Foo.method (file:///home/jamesernator/projects/playground/test.js:68:27)
    at Foo.method [as method] (file:///home/jamesernator/projects/playground/test.js:55:28)
    at file:///home/jamesernator/projects/playground/test.js:82:9

Is this really more desirable than?:

Error: some error
    at Foo.method (file:///home/jamesernator/projects/playground/test.js:68:27)
    at Foo.decoratedMethod [as method] (file:///home/jamesernator/projects/playground/test.js:55:28)
    at file:///home/jamesernator/projects/playground/test.js:82:9

[ljharb]

Sure, it would need to also only set the InitialName slot when it was previously unset - and arguably it's already missing an assertion that it's not previously set.

[rbuckton]

I'm in agreement that setting the name automatically is a bad idea. Yes, there's a negative impact to the debugging experience if the function name isn't set, but that's already the case when "monkey patching" existing methods. It may be cumbersome, but I would still much rather a developer be explicit about naming if it is important to their use case, i.e.:

function setFunctionName(f, name, prefix) {
  if (typeof name === "symbol") name = `[${name.description}]`;
  if (prefix) name = `${prefix} ${name}`;
  Object.defineProperty(f, "name", { configurable: true, value: name });
  return f;
}

function decorate(target, context) {
  return setFunctionName(function () { return target.apply(this, arguments); });
}

// or

function decorate(target, context) {
  // hacky way to assign name
  return { [context.name]: function () { return target.apply(this, arguments); } }[context.name];
}

[Jamesernator]

For some comparable behaviour in another language, in Python names are not carried over automatically, rather there's a builtin facility to copy the name (and docstring) over:

import functools

def decorate(method):
    @functools.wraps(method)
    def decorated_method(self):
        return method()
    return new_method

class Test:
    @decorate
    def method():
        raise Exception("some error")

test = Test()
# The name is updated
print(test.method.__name__)
test.method()

Interestingly though in this example, within a stack trace the original name of the function is still shown even though __name__ is changed:

Traceback (most recent call last):
  File "/home/jamesernator/projects/playground/test.py", line 16, in <module>
    test.method()
  File "/home/jamesernator/projects/playground/test.py", line 6, in decorated_method
    return method()
  File "/home/jamesernator/projects/playground/test.py", line 12, in method
    raise Exception("some error")
Exception: some error

Given the fairly flexible ways engines display stack traces in JS already, I suppose it would be already be allowed for engines to show more useful information anyway. e.g. Perhaps engines could show something like the following:

Error: some error
    at method [original Foo.method] (file:///home/jamesernator/projects/playground/test.js:68:27)
    at decoratedMethod [as Foo.method] (file:///home/jamesernator/projects/playground/test.js:55:28)
    at file:///home/jamesernator/projects/playground/test.js:82:9

[pzuraq]

This has been resolved, SetFunctionName is no longer called.