Using rest destructing on class objects seems to include getters in the resulting type

Bug Report

If you have a class along the lines of:

class User {
    constructor(
        public uid: number,
        public password: string // sensitive info
    ) { }
	
    get score(): number {
        return 10; // example code - imagine having to fetch something a little more complex
    }
}

If we then want to destructure a User object (e.g. returning a client-friendly basic object from a server), we first declare a type:

type UserClient = Omit<User, 'password' | 'score'>;

We pass score to the Omit here because, as we're using destructing, only basic properties will be returned, not any getters. We can then create our method:

toClient(): UserClient {
    const { password, ...client } = this;
    return client;
}

However, score is something that we want to return, so we change our UserClient type to:

type UserClient = Omit<User, 'password' | 'score'> | { score: number };

and our toClient() method now becomes:

toClient(): UserClient {
    const { password, ...client } = this;
    return {
        score: this.score,
        ...client
    }
}

This, however, gives the error 'score' is specified more than once, so this usage will be overwritten, even though when we destructure an object, it doesn't give us the getters - it seems like the inferred type from this line:

const { password, ...client } = this;

is wrong (specifically, it's Omit<this, 'password'> for client). In order to fix it, I need to change that line to

const { password, score, ...client } = this;

This doesn't really have any effect on the underlying JavaScript code, so it's a minor bug (I think)

🔎 Search Terms

rest
destructuring

🕗 Version & Regression Information

I'm currently on 4.4.3 though running in TypeScript Playground, 3.9.7 is when it first seems to show up.

In that version, the inferred type is Pick<this, Exclude<keyof this, "password">>

In 3.8.3 the inferred type is the same, though the error isn't present, so there must have been a change to the underlying Pick code (perhaps to include getters?)

This changed between versions 3.8.3 and 3.9.7

⏯ Playground Link

https://www.typescriptlang.org/play?ts=4.4.4#code/MYGwhgzhAECqEFMBO0DeAoAkMA9gOwgBckBXYQnJACi0wAcSAjEAS2GhJYBMAuaPEgFtGyADS0GzNtDqQIAd0q9oRJCzwBzaAHptKhARaEWANwTR1AMxxYAlGmgBfLFg0JCK3EgRVbfAcLIaLTehCRIeNAAjAAMANw6eggAHmCCdCDmuFzmALQWgmAa6uYAFmAm6loU0JbuwKUqOILupVXQYNCshISZ0IKUWc0ZKVjO6NCT0BQAwqwGhL588MhzLAvBU1vQuAQeqDJyikhcotAAdJeg63gejtAAvNNtEHET25Oh4ZEYHx8QXgQfEIL3OAMG4j+20u52uC3eH2cW2czkIAE86OYVkg1hsngB5QRGAA82LOAHJZFBjlxydAAD7Qcng7zkgB8DIcLKB-CEIhQjjiQA

💻 Code

class User {
    constructor(
        public uid: number,
        public password: string // sensitive info
    ) { }
	
    get score(): number {
        return 10; // example code - imagine having to fetch something a little more complex
    }

    toClient(): UserClient {
        const { password, ...client } = this;
        return {
            score: this.score, // ERROR: 'score' is specified more than once, so this usage will be overwritten
            ...client
        }
    }
}
type UserClient = Omit<User, 'password' | 'score'> | { score: number };

🙁 Actual behavior

The inferred type on client seems to include getters

🙂 Expected behavior

The inferred type on client to only include basic properties as it's created through destructuring

Workaround: Use the specified class type instead of the polymorphic this type

toClient() {
    const { password, ...client } = this as User 
    return {
        score: this.score, // OK
        ...client
    }
}

whzx5byb

whzx5byb Can you explain that a bit, please? Why is this not already considered a User inside it's own class method?

Why is this not already considered a User inside it's own class method?

this is considered generic, i.e. this extends User, because your method may be called through a subclass.

Thanks for giving a version range where this stopped working; I was able to bisect this to #37192.

Hm, that PR is actually just what expanded out the error message; the core issue that rest destructuring this doesn't omit properties seems to have existed for much longer (potentially always).

Here's a modified example that shows this behavior:

class User {
	constructor(
		public uid: number,
		public password: string // sensitive info
	) { }
	
	get score(): number {
		return 10; // example code - imagine having to fetch something a little more complex
	}

    toClient(): { uid: number; score?: never } {
        const { password: _, ...client } = this;
        return client; // Type 'this["score"]' is not assignable to type 'undefined'.
    }

    toClient2(): { uid: number; score?: never } {
        const { password: _, ...client } = this as User;
        return client; // OK
    }
}

https://www.typescriptlang.org/play?ts=4.5.2#code/MYGwhgzhAECqEFMBO0DeAoAkMA9gOwgBckBXYQnJACi0wAcSAjEAS2GhJYBMAuaPEgFtGyADS0GzNtDqQIAd0q9oRJCzwBzaAHptKhARaEWANwTR1AMxxYAlGmgBfLFg0JCK3EgRVbfAcLIaLTehCRIeNAAjAAMANw6eggAHmCCdCDmuFzmALQWgmAa6uYAFmAm6loU0JbuwKUqOILupVXQYNCshISZ0IKUWc0ZKVjO6NCT0BQAwqwGhL58qBzc-kIiSAkQXggA-P4IZiiOwVPn0LgEHiuyUIpIygD6otAAdB+gLAtO0AC80zaEDiEwuk1C4UiXwWIPO43Os3meEIACYlg5OMoApttrsDvwjkFThgwZMrkQHHcFEo+C93p8kR5TgDCECOjB4MhYaSIRFLozuZNnM4gA

With the desired behavior being that both toClient and toClient2 have no errors, but as User is the only one that works consistently.

I was able to "fix" the original case by not using Omit on this when destructuring, however one can also destructure a value of type T extends SomeClassWithPropGetter and get the same thing. The extended test case is:

class A {
    constructor(public foo: string) {}

    get bar(): number {
        return 1;
    }

    func() {
        const {           ...rest1 } = this;
        const {           ...rest2 } = this as A;
        const { foo: _f1, ...rest3 } = this;
        const { foo: _f2, ...rest4 } = this as A;

        // Rest destructuring drops properties provided by getters.
        // "bar" should not be present in any of these.
        rest1.bar;
        rest2.bar;
        rest3.bar;
        rest4.bar;
    }
}

function destructure<T extends A>(x: T) {
    const {           ...rest1 } = x;
    const {           ...rest2 } = x as A;
    const { foo: _f1, ...rest3 } = x;
    const { foo: _f2, ...rest4 } = x as A;

    // Rest destructuring drops properties provided by getters.
    // "bar" should not be present in any of these.
    rest1.bar;
    rest2.bar;
    rest3.bar;
    rest4.bar;
}

Playground Link

Where all 8 accesses to bar should be errored.

Jake Bailey (@jakebailey)

Hi Jake Bailey (@jakebailey) ,

Thanks for looking at this, and getting a fix so quickly.

To be clear, in your last Playground link, it'll error on all 8 access to bar in a future TypeScript version? In v4.5.2, it only errors out on the 4 as A lines for me

That's what my PR does, yes. You can try out my PR here: Playground Link

There's unfortunately a little more nuance that my PR needs to handle better, namely, it should probably be legal to destructure non-public properties when it's this of the current class being unpacked; mine disallows this but probably only because I'm reusing the code that previously only ran on the non-generic case, where you're only considering the public interface of a type.

I'll make sure we discuss it before merging.

Per #47078 (comment), dropping non-public seems correct, so the behavior I'm showing is the final one.

Jake Bailey (@jakebailey)

Thanks again, Jake Bailey (@jakebailey) and Ryan Cavanaugh (@RyanCavanaugh) for the quick turnaround! 👍

as we're using destructing, only basic properties will be returned, not any getters.

This seems like an unsafe assumption. Whether a property is copied into a rest pattern depends on whether it is an own property, not on whether a property is a getter or a method. As far as I understand, TypeScript doesn't distinguish own from inherited properties?

Navigation Menu

Repository navigation

Using rest destructing on class objects seems to include getters in the resulting type #46967

Bug Report

🔎 Search Terms

🕗 Version & Regression Information

⏯ Playground Link

💻 Code

🙁 Actual behavior

🙂 Expected behavior

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Using rest destructing on class objects seems to include getters in the resulting type #46967

Description

Bug Report

🔎 Search Terms

🕗 Version & Regression Information

⏯ Playground Link

💻 Code

🙁 Actual behavior

🙂 Expected behavior

Activity

whzx5byb commented on Dec 1, 2021

divillysausages commented on Dec 1, 2021

fatcerberus commented on Dec 4, 2021

jakebailey commented on Dec 7, 2021

jakebailey commented on Dec 7, 2021

jakebailey commented on Dec 9, 2021

divillysausages commented on Dec 9, 2021

jakebailey commented on Dec 9, 2021

jakebailey commented on Dec 9, 2021

divillysausages commented on Dec 10, 2021

bergus commented on Jan 26, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions