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

Setting enumerable: false within a property decorator does not work as expected #22048

Closed
JonWallsten opened this issue Feb 20, 2018 · 4 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@JonWallsten
Copy link

JonWallsten commented Feb 20, 2018

TypeScript Version: 2.7.1-insiders.20180127 (TypeScript Playground)

Search Terms:
typescript decorator descriptor enumerable writable defineProperty instance

Code

Example 1

Setting enumerable: false with a decorator

const nonenumerable = (target: any, propertyKey: string) => {
    let descriptor = Object.getOwnPropertyDescriptor(target, propertyKey) || {};
    if (descriptor.enumerable !== false) {
        descriptor.enumerable = false;
        Object.defineProperty(target, propertyKey, descriptor)
    }
}

class Employee {
    @nonenumerable
    public id: number;
    public name: string;

    constructor() { 
        this.id = 1;
        console.log("this.id | Expected: 1 | Actual: ", this.id);
    }
}

var user = new Employee();
user.id = 2;
user.name = 'John Doe';
console.log("user.id | Expected: 2 | Actual: ", user.id);
console.log("Object.keys(user).length | Expected: 1 | Actual: ", Object.keys(user).length);

console.log("Object.keys(user): ", Object.keys(user));

Expected behavior:
The id property should not be visible when iterating the keys and the value should be 1 when logged in the constructor and 2 when logged after the value is changed.

Actual behavior:
The id property is indeed not visible when iterating the keys but the value is undefined when logged on both occurrences.

Playground Link:
https://goo.gl/oh9PH1

Second example

Setting enumerable: false and writable: true with a decorator

const nonenumerable = (target: any, propertyKey: string) => {
    let descriptor = Object.getOwnPropertyDescriptor(target, propertyKey) || {};
    if (descriptor.enumerable !== false) {
        descriptor.enumerable = false;
        descriptor.writable = true;
        Object.defineProperty(target, propertyKey, descriptor)
    }
}

class Employee {
    @nonenumerable
    public id: number;
    public name: string;

    constructor() { 
        this.id = 1;
        console.log("this.id | Expected: 1 | Actual: ", this.id);
    }
}

var user = new Employee();
user.id = 2;
user.name = 'John Doe';
console.log("user.id | Expected: 2 | Actual: ", user.id);
console.log("Object.keys(user).length | Expected: 1 | Actual: ", Object.keys(user).length);

console.log("Object.keys(user): ", Object.keys(user));

Expected behavior:
The id property should not be visible when iterating the keys and the value should be 1 when logged in the constructor and 2 when logged after the value is changed.

Actual behavior:
The id property is visible when iterating the keys. The value is 1 when logged in the constructor and 2 when logged after it's changed.

Playground Link:
https://goo.gl/8L662Q

Third example (works as expected)

Setting enumerable: false and writable: true within the constructor

const nonenumerable = (target: any, propertyKey: string) => {
    let descriptor = Object.getOwnPropertyDescriptor(target, propertyKey) || {};
    if (descriptor.enumerable !== false) {
        descriptor.enumerable = false;
        descriptor.writable = true;
        Object.defineProperty(target, propertyKey, descriptor)
    }
}

class Employee {
    public id: number;
    public name: string;

    constructor() { 
        nonenumerable(this, 'id');
        this.id = 1;
        console.log("this.id | Expected: 1 | Actual: ", this.id);
    }
}

var user = new Employee();
user.id = 2;
user.name = 'John Doe';
console.log("user.id | Expected: 2 | Actual: ", user.id);
console.log("Object.keys(user).length | Expected: 1 | Actual: ", Object.keys(user).length);

console.log("Object.keys(user): ", Object.keys(user));

Expected behavior:
The id property should not be visible when iterating the keys and the value should be 1 when logged in the constructor and 2 when logged after the value is changed.

Actual behavior:
The id property is not visible when iterating the keys and the value is 1 when logged in the constructor and 2 when logged after the value is changed.

Playground Link:
https://goo.gl/kFvycX

@JonWallsten JonWallsten changed the title Settings enumerable: false within a property decorator does not work as expected Setting enumerable: false within a property decorator does not work as expected Feb 20, 2018
@aluanhaddad
Copy link
Contributor

This is expected because the decorated property is placed on the prototype and not the instance.

@JonWallsten
Copy link
Author

JonWallsten commented Feb 22, 2018

@aluanhaddad Yeah, I can tell as much by looking at the code and it might be expected technically due to the implementation, but for me as a developer it's really not. So the questions remains, is this the intended behavior, or a current limitation?

@mhegazy mhegazy added the Working as Intended The behavior described is the intended behavior; this is not a bug label Feb 22, 2018
@mhegazy
Copy link
Contributor

mhegazy commented Feb 22, 2018

This behaves per spec. it is worth noting that the current Decorator ES spec has moved away from this design, and TS will need to implement the new proposal.

@JonWallsten
Copy link
Author

Thank you for the info, @mhegazy. We'll skip this implementation for now and wait until TS is ready then.

@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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