microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.17k stars 12.38k forks source link

TypeScript does not set function name for MethodDeclarations #5611

Closed azz closed 6 months ago

azz commented 8 years ago

This issue has been asked as a question on StackOverflow.

I have a TypeScript class, and I'm doing some metaprogramming where I need to be able to access instance.func.name, however TypeScript omits the function name in the compiled JS.

TypeScript:

class ClassName {
    // ... 
    func(): ReturnType {
        // ...
    }
}

Compiled JavaScript:

// ...
ClassName.prototype.func = function () {
    // ... 
};

Desired JavaScript:

ClassName.prototype.func = function func() {
    // ...                          ^^^^
};

Not sure if this is a bug or by design, but the source of the result is emitter.ts.

A proposed solution on StackOverflow was:

// src/compiler/emitter.ts:4671
function shouldEmitFunctionName(node) {
    if (node.kind === 173 /* FunctionExpression */) {
        // Emit name if one is present
        return !!node.name;
    }
    if (node.kind === 213 /* FunctionDeclaration */) {
        // Emit name if one is present, or emit generated name in down-level case (for export default case)
        return !!node.name || languageVersion < 2 /* ES6 */;
    }

    // MODIFIED
    if (node.kind === 143 /* MethodDeclaration */) {                    
        return true;
    }                                                        
}
RyanCavanaugh commented 8 years ago

We need to do this for ES6 compat

RyanCavanaugh commented 8 years ago

Open question -- what should happen when the function name is a quoted name that is an invalid identifier?

class Thing {
  'foo bar'() { }
}

var x = new Thing();
console.log(x['foo bar'].name);

ES6 prints foo bar, Babel prints fooBar (wat), TypeScript prints undefined.

mhegazy commented 8 years ago

set it explicitly?...

Object.defineProperty(Thing.prototype["foo bar"], "name", { configurable: true, writable: false, enumerable: false, value:"foo bar" });
DanielRosenwasser commented 8 years ago

You'd basically have to do the same thing we do for computed properties in object literals and split off at the first property with a string name whose name is not a valid identifier name.

jeffreymorlan commented 8 years ago

Another time using a named function wouldn't work is when the method accesses an outer variable with the same name:

var func;
class ClassName {
    func() { return func; }
}

Edit: a named function could also be incorrectly fetched by eval:

var func;
class ClassName {
    func(s) { return eval(s); }
}
new ClassName().func('func');
MartyIX commented 8 years ago

@jeffreymorlan has a point.

var func = "aa";
class ClassName {
    func() { return func; }
}
new ClassName().func();

returns "aa" in Chrome console

var func = "aa";
var ClassName = (function () {
    function ClassName() {
    }
    ClassName.prototype.func = function func() { return func; };
    return ClassName;
})();
new ClassName().func();

returns function func() { return func; } in Chrome console.

LPGhatguy commented 8 years ago

@jeffreymorlan The outer element with the conflicting name could be aliased under a different variable name.

I ran into this issue and I see that it was pushed back from several versions. Is there a desire for this feature?

azz commented 8 years ago

@LPGhatguy even if aliasing were possible, that doesn't solve the eval case.

To avoid the issues pointed out by @jeffreymorlan, would the emitter have to emit Object.defineProperty(...) for all methods? If so:

  1. It would probably compromise TypeScript's "compiles to clean JavaScript output" slogan.
  2. It wouldn't work on ES3 targets.

This complexity explains why the issue has been pushed back. That being said, I would like to see it solved. As @RyanCavanaugh pointed out, it is needed for ES2015 compatibility.

I suppose a __methodNames helper could be emitted at the top of the file:

// ES5 emit:
var __methodNames = (this && this.__methodNames) || function (proto, keys) {
    for (var i = keys.length - 1; i >= 0; i--) {
        Object.defineProperty(proto[keys[i]], 'name', { 
            value: keys[i], configurable: true, writable: false, enumerable: false
        });
    }
};

Emitting for methods, similar to how __decorate works:

ClassName.prototype.foo = function () {
   // ...
};
ClassName.prototype['bar baz'] = function () {
   // ...
};
 __methodNames(ClassName.prototype, ['foo', 'bar baz']);
AllNamesRTaken commented 7 years ago

Is this planned for version 2.3 now? It really ruined the day for my test framework and will be loved when it comes.

Jack-Works commented 6 years ago

Any progress on it?

Manish3177 commented 6 years ago

Hello, are there any updates on this issue? According to the latest activity above (on 8/31/2017), it was supposed to be added to TypeScript 2.6 but it isn't there in it. Also, if it's not too late, I would like to suggest that the name should include the class name as well (ClassName$functionName, for example). In large projects where the same function is used by several classes (for example when overriding a function from a base class), the function name alone isn't sufficient to uniquely identify the actual function. It would be even better if there was a way to control the prefix.

jannesiera commented 6 years ago

Bump

nwayve commented 6 years ago

This seems to be working for me just fine (at least for TypeScript v2.9.2).

Be sure that your tsconfig.json file (or however you're setting your compilerOptions) has compilerOptions.target set to "es2015" | "es6" or later.

Using ts-node with target >= "es2015":

> class Foo {
... bar() {}
... }
> let foo = new Foo();
undefined
> foo.bar.name
'bar'
> foo.constructor.name
'Foo'

I threw that last bit in there for @Manish3177 . No need to include the class name in the method name as it can be retrieved from the constructor object on the class instance.

weswigham commented 6 years ago

Be sure that your tsconfig.json file (or however you're setting your compilerOptions) has compilerOptions.target set to "es2015" | "es6" or later.

That just means we're not downleveling the method declaration. The issue is for the downleveling targets.

FreePhoenix888 commented 12 months ago

+1. The main reason why I need name for methods:

    async convert() {
      const log = ObjectToLinksConverter.classLog.extend(this.convert.name);
    }
FreePhoenix888 commented 12 months ago

Workaround/Possible Soltuion

I have described the problem on this stackoverflow question and written the solution

FreePhoenix888 commented 12 months ago

Solution

I have described the problem on this stackoverflow question and written the solution

@jannesiera @Manish3177 @Jack-Works @ob3c @pinyin @sonicd300 @elado @jfahrenkrug @Fallenstedt @joseph-of-wheel @a-tarasyuk @MattiasBuelens

RyanCavanaugh commented 6 months ago

This doesn't seem like an important scenario for modern JS development - folks can either target ES6 which is supported basically everywhere, or use an alternate transpiler with stricter adherence semantics if this is important for their scenario.