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

Way of specifying non-enumerable properties #9726

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

Way of specifying non-enumerable properties #9726

tinganho opened this issue Jul 14, 2016 · 17 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@tinganho
Copy link
Contributor

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 RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature and removed In Discussion Not yet reached consensus labels Aug 18, 2016
@RyanCavanaugh
Copy link
Member

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

@jwgmeligmeyling
Copy link

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.

@clshortfuse
Copy link

clshortfuse commented Jul 13, 2020

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

@michaelcheers
Copy link

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

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 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?

@scottmas
Copy link

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

@e1himself
Copy link

e1himself commented Oct 21, 2021

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!

@DetachHead
Copy link
Contributor

DetachHead commented Apr 14, 2022

yeah this definitely doesn't seem like an edge case to me

const foo: string[] = []

//error: Type '{}' is missing the following properties from type 'string[]': length, pop, push, concat, and 26 more.
const bar: string[] = {}

//no error
const baz: string[] = {...foo}

//no error, fails at runtime
baz.push("")

@mixtur
Copy link

mixtur commented Sep 2, 2022

An ability to make assertions on enumerability is essential. This is a source of many bugs, and if a little birdie told us that we are doing something suspicious here

const v: IVector = new Vector();
const v2: IVector = { ...v };
v2.x += 3;// x is a non-enumerable setter / getter

...our users would be slightly less sad.

I do not even insist on nonenum modifier. Maybe the opposite in meaning say "enum" is enough. That way you can make it strict only when you want it to be strict.

That is what we actually have. We expose plain-object API to users, but internally we use classes with getters, and sometimes people on the team can make this mistake. So if we were able to require either plainobjectness on interfaces or enumerability on properties it would help.

@vasyop
Copy link

vasyop commented Oct 6, 2022

#51073 (comment)

@adrm
Copy link

adrm commented Jan 7, 2023

Anyone could share at least a workaround? I am stuck since non-enumerable props should be dropped from the type when I do an object spread, but should be there until I do the spread. I have found no way to control this with the type system at compile time but it is happening on runtime.

@e1himself
Copy link

Anyone could share at least a workaround?

I don't think there is a workaround. It's just not possible with Typescript type system.

@MichaelScheffenacker
Copy link

Why is the issue of the type checker not recognizing the outer type (i.e. {} instead of []) tracked by this thread, that started with an issue of Object.assign().

I could imagine that the Object.assign() Problem might cause the outer type problem in some tangled ways, but would the latter not still be much easier to fix than the former? The type checker would just have to look at the outer type (Array or Object literal) to decide that there is a type mismatch. The inner type is checked correctly anyways, why not check the outer type too? (Sorry, if I have a simplistic view about it, but currently I have no deeper insight about the type checking constraints there ...)

@snarfblam
Copy link

Anyone could share at least a workaround? I am stuck since non-enumerable props should be dropped from the type when I do an object spread, but should be there until I do the spread. I have found no way to control this with the type system at compile time but it is happening on runtime.

You could convey non-enumerability by using a 'flavor' type. Of course this doesn't help with pre-existing library types or the results of language features (e.g. spread operator or instance types of classes), but you can strip away non-enumerable-flavored properties with a helper type (EnumerableOnly<T>).

/**** nonenum Flavor *****/

// Tag type ('flavor') to mark non-enumerable properties
type nonenum = {[nonEnumSym]?: typeof nonEnumSym};


/**** Helpers *****/

// Unique symbol for the above
declare const nonEnumSym: unique symbol;
// Utility type excludes non-enumerable members
type EnumerableOnly<T> = Omit<T,keyof FilterByType<T, nonenum>>;
// Utility type to pick only properties whose type matches a specified type/shape
type FilterByType<T, Value> = {
  [P in keyof T as T[P] extends Value | undefined ? P : never]: T[P]
}


/**** Usage *****/

// Includes enumerable and non-enumerable types
interface NonEnumExample {
  x: number;
  y: number;
  name: nonenum & string;
  format: nonenum & (() => string);
}

type WithoutNonenums = EnumerableOnly<NonEnumExample>;

Complete example: Playground link (includes an Object.assign facsimile that strips non-enumerable properties)

@c-vetter
Copy link

@jwgmeligmeyling

… I'm not sure though if the enumsof is really relevant though.

How would you construct the proper output type without enumsof? That is, how would you write assign<T, U>(target: T, source: U): T & enumsof U;? Plus, without enumsof, what's the value of nonenum?

@mixtur

I do not even insist on nonenum modifier. Maybe the opposite in meaning say "enum" is enough. That way you can make it strict only when you want it to be strict.

I think an enum marker would be at least as difficult to implement as nonenum, if not more so. More importantly, since enumerability is the default and you have to do extra work to produce non-enumerable properties, it makes sense to make nonenumerability the annotated variant – both for the overall amount of writing/reading, and for making both code and types grow at the same time for the same reasons.

@adrm

Anyone could share at least a workaround? I am stuck since non-enumerable props should be dropped from the type when I do an object spread, but should be there until I do the spread. I have found no way to control this with the type system at compile time but it is happening on runtime.

The main way is to assert this manually, which is obviously painful and errorprone. For a more robust solution, see @snarfblam's reply as well as my addition below 👇👇

@MichaelScheffenacker

Why is the issue of the type checker not recognizing the outer type (i.e. {} instead of []) tracked by this thread, that started with an issue of Object.assign().

I could imagine that the Object.assign() Problem might cause the outer type problem in some tangled ways, but would the latter not still be much easier to fix than the former? The type checker would just have to look at the outer type (Array or Object literal) to decide that there is a type mismatch. The inner type is checked correctly anyways, why not check the outer type too? (Sorry, if I have a simplistic view about it, but currently I have no deeper insight about the type checking constraints there ...)

The answer to this lies in the nature of TypeScript's type system. TS does not employ nominal types but matches structurally only.

See this trivial example:

type User = { id: number }
type Media = { id: number }
const user : User = { id: 1 }
const media : Media = user // works fine

Using nominal types, line 4 would have an error, but it doesn't because the shape/structure fits. I you come from a system with nominal types, you may expect stronger guarantees, but this makes a lot of sense in the context of TypeScript being JavaScript with types and needing to compile to JavaScript with minimal transformation. See https://github.com/Microsoft/TypeScript/wiki/TypeScript-Design-Goals for details.

@snarfblam

…You could convey non-enumerability by using a 'flavor' type. Of course this doesn't help with pre-existing library types or the results of language features (e.g. spread operator or instance types of classes), but you can strip away non-enumerable-flavored properties with a helper type (EnumerableOnly<T>)…

Nice, gets pretty close. Unfortunately, the flavor sticks to the value, not the key. Therefore, at least one more operation would be required to enable pushing that data out in a “clean” state. Another utility like this would do it:

type CleanNonEnumerables<T> = {
	[P in keyof T]: T[P] extends nonenum & infer V ? V : T[P]
}

With that, a cleaned type can be generated.

type Cleaned = CleanNonEnumerables<NonEnumExample>

@iameli
Copy link

iameli commented Mar 12, 2024

Is this issue why you can spread whatever properties you like onto objects that don't support them? TypeScript has no problem with this, apparently:

type Person = {
  name: string;
};

type Pie = {
  flavor: string;
};

let eli: Person = { name: "Eli" };
let applePie: Pie = { flavor: "Apple" };

const weirdMutant: Person = {
  ...eli,
  ...applePie,
};

console.log(weirdMutant)

I'm used to doing lots of inline spreads in things like Redux reducers and was surprised TypeScript couldn't help me out here.

EDIT: Ah, I think I'm looking for #12936

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests