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

Class name not being inherited in anonymous classes results in decorated classes losing their original name with Node 12.16.0 and above #37157

Closed
plameniv opened this issue Mar 2, 2020 · 2 comments
Assignees
Labels
Rescheduled This issue was previously scheduled to an earlier milestone Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@plameniv
Copy link

plameniv commented Mar 2, 2020

TypeScript Version: 3.6.2, 3.8.2

Search Terms: decorator, anonymous class

Expected behavior:
Using a decorator which modifies the constructor of a class returns a class with the same name as the original class.

Using more than one decorator which modify the constructor of a class returns a class whose prototype's name is the original class's name.

Actual behavior:
Using a decorator which modifies the constructor of a class replaces the name of the class with the empty string.

Using more than one decorator which modify the constructor of a class also replaces the name of the prototype of the class with the empty string.

Additional details:
Node 12.16.0 introduces a change which makes the .name property of an anonymous class the empty string: #31830.

From a comment in that issue:

The V8 change that's responsible was reverted because of fuzzer issues and hasn't been relanded, AFAICT.

However, in the future that change may still come back to V8 and continue affecting the decorator behavior.

Code
Replicator:

function classDecorator<T extends new (...args: any[]) => {}>(Tconstructor: T) {
    return class extends Tconstructor {
        public var = "override";
    };
}

class NotDecorated {
    public var: string;
    constructor(m: string) {
        this.var = m;
    }
}

@classDecorator
class OnceDecorated {
    public var: string;
    constructor(m: string) {
        this.var = m;
    }
}

@classDecorator
@classDecorator
class TwiceDecorated {
    public var: string;
    constructor(m: string) {
        this.var = m;
    }
}

function classDecoratorWithName<T extends new (...args: any[]) => {}>(Tconstructor: T) {
    return class ClassName extends Tconstructor {
        public var = "override";
    };
}

@classDecoratorWithName
class NamedDecorated {
    public var: string;
    constructor(m: string) {
        this.var = m;
    }
}

function sealed(constructor: Function) {
    Object.seal(constructor);
    Object.seal(constructor.prototype);
}

@sealed
@sealed
class SealedClass {
    public var: string;
    constructor(m: string) {
        this.var = m;
    }
}

console.log("");
console.log(`NotDecorated.name:             "${NotDecorated.name}"`);
console.log(`NotDecorated.prototype.name:   "${Object.getPrototypeOf(NotDecorated).name}"`);
console.log("");
console.log(`OnceDecorated.name:            "${OnceDecorated.name}"`);
console.log(`OnceDecorated.prototype.name:  "${Object.getPrototypeOf(OnceDecorated).name}"`);
console.log("");
console.log(`TwiceDecorated.name:           "${TwiceDecorated.name}"`);
console.log(`TwiceDecorated.prototype.name: "${Object.getPrototypeOf(TwiceDecorated).name}"`);
console.log("");
console.log(`NamedDecorated.name:           "${NamedDecorated.name}"`);
console.log(`NamedDecorated.prototype.name: "${Object.getPrototypeOf(NamedDecorated).name}"`);
console.log("");
console.log(`SealedClass.name:              "${SealedClass.name}"`);
console.log(`SealedClass.prototype.name:    "${Object.getPrototypeOf(SealedClass).name}"`);
console.log("");

Expected result with 12.16.0:

NotDecorated.name:             "NotDecorated"
NotDecorated.prototype.name:   ""

OnceDecorated.name:            ""
OnceDecorated.prototype.name:  "OnceDecorated"

TwiceDecorated.name:           ""
TwiceDecorated.prototype.name: ""

NamedDecorated.name:           "ClassName"
NamedDecorated.prototype.name: "NamedDecorated"

SealedClass.name:              "SealedClass"
SealedClass.prototype.name:    ""

Expected result with 12.15.0:

NotDecorated.name:             "NotDecorated"
NotDecorated.prototype.name:   ""

OnceDecorated.name:            "OnceDecorated"
OnceDecorated.prototype.name:  "OnceDecorated"

TwiceDecorated.name:           "TwiceDecorated"
TwiceDecorated.prototype.name: "TwiceDecorated"

NamedDecorated.name:           "ClassName"
NamedDecorated.prototype.name: "NamedDecorated"

SealedClass.name:              "SealedClass"
SealedClass.prototype.name:    ""

The following is the tsconfig that was used:

{
  "compileOnSave": true,
  "compilerOptions": {
    "sourceMap": true,
    "module": "commonjs",
    "target": "es6",
    "lib": ["es2017", "esnext"],
    "strict": true,
    "strictNullChecks": false,
    "noImplicitAny": false,
    "noImplicitThis": true,
    "declaration": true,
    "noEmitOnError": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "experimentalDecorators": true,
    "forceConsistentCasingInFileNames": true
  },
}

The following comment provided a possible workaround: nodejs/node#32035 (comment)

Decorator function which loses original name:

function classDecorator<T extends new (...args: any[]) => {}>(Tconstructor: T) {
    return class extends Tconstructor {
        public var = "override";
    };
}

Example decorator function which preserves the original name:

function classDecorator<T extends new (...args: any[]) => {}>(Tconstructor: T) {
    const temp = class extends Tconstructor {
        public var = "override";
    };
    Object.defineProperty(temp, "name", { value: Tconstructor.name });
    return temp;
}

Playground Link:

Related Issues:

@rbuckton
Copy link
Member

rbuckton commented Jul 7, 2020

I can't speak to the V8 behavior around Function.prototype.name, but decorators in TypeScript do not do anything to affect the name of the resulting class. Decorator application is essentially a function call pipeline: @A @B class C {} is essentially the same as let C = A(B(class C {})). Class decorators don't "modify the constructor of a class", but rather replace the class with another class. In your classDecorator above, you replace the value of Tconstructor with your own anonymous class expression class extends Tconstructor { ... }.

The only way to preserve the name of the original class when replacing it in this fashion is to do so manually:

Here's an example of using NamedEvaluation:

function classDecorator<T extends new (...args: any[]) => {}>(Tconstructor: T) {
    return {
        [Tconstructor.name]: class extends Tconstructor {
            public var = "override";
        }
    }[Tconstructor.name];
}

In the above example, we create an object literal with a property whose name is the value of Tconstructor.name. ES2015's NamedEvaluation will use that name to name the class expression. However, this won't work when targeting ES5/ES3 as TypeScript does not currently downlevel ES2015's NamedEvaluation behavior to ES5/3.

@rbuckton rbuckton added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Needs Investigation This issue needs a team member to investigate its status. labels Jul 7, 2020
@rbuckton
Copy link
Member

rbuckton commented Jul 7, 2020

I'll also note that using a decorator that replaces the class results in the following prototype chain:

// @classExpression class OnceDecorated {}
(class expression)           --> OnceDecorated
(class expression).prototype --> OnceDecorated.prototype

// @classExpression @classExpression TwiceDecorated { }
(class expression #2)           --> (class expression #1)           --> TwiceDecorated
(class expression #2).prototype --> (class expression #1).prototype --> TwiceDecorated.prototype

When you apply classDecorator once, you are essentially writing this:

(class extends (class OnceDecorated { }) { })

When you apply classDecorator twice, you are essentially doing this:

(class extends (class extends (class TwiceDecorated { }) { }) { }

There's no reason the result of the twice-decorated class should have a prototype whose name is TwiceDecorated, as the outermost unnamed class expression's [[Prototype]] is another unnamed class expression. Per the ECMAScript spec, name on a class should always be set (per 14.6.14 Runtime Semantics: ClassDefinitionEvaluation, step 13), so if V8 was failing to set the name of the unnamed class to the empty string then it likely was a bug in V8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rescheduled This issue was previously scheduled to an earlier milestone Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants