X Tutup
The Wayback Machine - https://web.archive.org/web/20220128165415/https://github.com/microsoft/TypeScript/issues/9726
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

Jump to bottom

Way of specifying non-enumerable properties #9726

Open
tinganho opened this issue Jul 14, 2016 · 8 comments
Open

Way of specifying non-enumerable properties #9726

tinganho opened this issue Jul 14, 2016 · 8 comments
Labels
Awaiting More Feedback Suggestion

Comments

@tinganho
Copy link
Contributor

@tinganho tinganho commented Jul 14, 2016

Object assign is defined like below in TS:

interface ObjectConstructor {
    /**
      * Copy the values of all of the enumerable own properties from one or more source objects to a
      * target object. Returns the target object.
      * @param target The target object to copy to.
      * @param source The source object from which to copy properties.
      */
    assign<T, U>(target: T, source: U): T & U;
}

Though from MDN it is only copying members that are enumerable:

The Object.assign() method is used to copy the values of all enumerable own properties from one or more source objects to a target object. It will return the target object.

So if U above has non-enumerable members, TS will copy them anyway. This is slightly incorrect and unsafe.

One issue I recently ran into was

const selection = Object.assign({}, window.getSelection());

I was assuming I was copying all members of the Selection object to {}:

interface Selection {
    readonly anchorNode: Node;
    readonly anchorOffset: number;
    // etc ...
}

declare var Selection: {
    prototype: Selection;
    new(): Selection;
}

Though it didn't copy any members at all, because the Selection object only contains non-enumerable members.

Proposal

Mark properties as non-enumerable

interface Selection {
    readonly nonenum anchorNode: Node;
    readonly nonenum anchorOffset: number;
    // etc ...
}

And have an operator to get the only "enum side" of a type:

interface ObjectConstructor {
    /**
      * Copy the values of all of the enumerable own properties from one or more source objects to a
      * target object. Returns the target object.
      * @param target The target object to copy to.
      * @param source The source object from which to copy properties.
      */
    assign<T, U>(target: T, source: U): T & enumsof U;
}
@RyanCavanaugh
Copy link
Member

@RyanCavanaugh RyanCavanaugh commented Aug 18, 2016

Seems like kind of an edge case; I wouldn't want to have to think about this for every .d.ts file for the sake of a couple of APIs

@DanielRosenwasser DanielRosenwasser changed the title Non-enums Way of specifying non-enumerable properties Aug 18, 2016
@RyanCavanaugh RyanCavanaugh mentioned this issue Aug 20, 2018
Closed
@RyanCavanaugh RyanCavanaugh mentioned this issue Feb 19, 2019
Closed
@RyanCavanaugh RyanCavanaugh mentioned this issue Jun 17, 2019
Closed
@RyanCavanaugh RyanCavanaugh mentioned this issue Jul 29, 2019
Closed
@AnyhowStep AnyhowStep mentioned this issue Aug 26, 2019
Closed
@RyanCavanaugh RyanCavanaugh mentioned this issue Oct 30, 2019
Closed
@RyanCavanaugh RyanCavanaugh mentioned this issue Dec 19, 2019
Closed
@jwgmeligmeyling
Copy link

@jwgmeligmeyling jwgmeligmeyling commented Mar 11, 2020

I'd like to see the nonenum keywaord as well. It's not only relevant for Object.assign, but also Object.keys(), Object.values(), JSON.stringify, etc. I'm not sure though if the enumsof is really relevant though.

@RyanCavanaugh RyanCavanaugh mentioned this issue May 29, 2020
Closed
@RyanCavanaugh RyanCavanaugh mentioned this issue Jun 8, 2020
Closed
@clshortfuse
Copy link

@clshortfuse clshortfuse commented Jul 13, 2020 •
edited

Might be useful for making Object.keys(T) return a filtered keyof T that excludes nonenum entries (by using enumsof ). Classes can have their methods always set to nonenum, which can help create remove some ambiguity between properties and methods. It essentially can predict the result of Object.prototype.hasOwnProperty.

Would help fix this old issue: #18133

@RyanCavanaugh RyanCavanaugh mentioned this issue Dec 7, 2020
Closed
@RyanCavanaugh RyanCavanaugh mentioned this issue Mar 16, 2021
Closed
@RyanCavanaugh RyanCavanaugh mentioned this issue Jun 10, 2021
Closed
@michaelcheers
Copy link

@michaelcheers michaelcheers commented Jun 11, 2021

Use case: https://stackoverflow.com/questions/67928288/typescript-get-names-of-fields-but-not-properties-or-at-least-get-only/67929115#67929115

Even if it can't be on interfaces, it would still be useful for classes to have by default.

@jwgmeligmeyling
Copy link

@jwgmeligmeyling jwgmeligmeyling commented Jul 20, 2021

I wouldn't want to have to think about this for every .d.ts file for the sake of a couple of APIs

Just want to take the opportunity to point out that with a nonenum keyword being introduced, I don't think necessarily all d.ts files would have to be revisited. This can be addressed over time.

@val-o
Copy link

@val-o val-o commented Sep 4, 2021

This issue is often referenced as an example of unsoundness of typescript. I wouldn't say it's an edge case as switching from objects to maps, array to sets are popular refactorings and at the same time usage of spreads with arrays and objects are ubiquitous. Shouldn't we prioritize this?

@MartinJohns MartinJohns mentioned this issue Sep 30, 2021
Open
@scottmas
Copy link

@scottmas scottmas commented Oct 11, 2021

@RyanCavanaugh Any update on the Typescript team's thinking on this? Right now it's pretty painful to work around this.

@kamilmielnik kamilmielnik mentioned this issue Oct 21, 2021
Merged
@e1himself
Copy link

@e1himself e1himself commented Oct 21, 2021 •
edited

Hi everyone 👋

Seems like kind of an edge case; I wouldn't want to have to think about this for every .d.ts file for the sake of a couple of APIs

Just to share an example that is not an edge case. Our team has lost time debugging a problem caused by this, while Typescript was perfectly happy with the code:

https://github.com/prezly/slate/pull/32/files#diff-b453474f430084c5f0426a814ca6bcb397b61b1bdc5576f777d1faff96486f36R78-R93

The problem was that spreading a DOMRect object does not work, as its properties are not enumerable. There's no way of catching it with Typescript, only debugging it in runtime.

I hope this helps to get a better perspective on this problem. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Awaiting More Feedback Suggestion
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
@e1himself @tinganho @scottmas @jwgmeligmeyling @RyanCavanaugh @clshortfuse @val-o @michaelcheers
X Tutup