microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.07k stars 12.37k forks source link

Way of specifying non-enumerable properties #9726

Open tinganho opened 8 years ago

tinganho commented 8 years ago

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 commented 8 years ago

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 commented 4 years ago

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 commented 4 years ago

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: https://github.com/Microsoft/TypeScript/issues/18133

michaelcheers commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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

e1himself commented 2 years ago

Hi everyone :wave:

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 commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

https://github.com/microsoft/TypeScript/issues/51073#issuecomment-1269778039

adrm commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 11 months ago

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 commented 6 months ago

@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 commented 5 months ago

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 https://github.com/microsoft/TypeScript/issues/12936