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

Object.create return type #3865

Closed
matthewjh opened this issue Jul 15, 2015 · 8 comments
Closed

Object.create return type #3865

matthewjh opened this issue Jul 15, 2015 · 8 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@matthewjh
Copy link

Hi,

Just curious as to the reasoning behind Object.create having a return value of type any in lib.d.ts.

Wouldn't this make more sense?

create<T extends A>(o: A, properties?: PropertyDescriptorMap): T;

since the returned object is guaranteed to have the properties of o through prototypal inheritance.

At present I'd like to create many objects (via literals) of type X, but given that many of the properties are shared, using Object.create makes a lot of sense. It seems a shame that I must use this at the expense of typing.

@RyanCavanaugh
Copy link
Member

Mostly we see Object.create being used in code that doesn't really benefit from typing, or with Object.create(null) to create a map object, which is going to require a type assertion anyway.

At present I'd like to create many objects (via literals) of type X, but given that many of the properties are shared, using Object.create makes a lot of sense

Can you post some examples of what this looks like? I can't think of any code that it would meaningfully benefit from a different declaration of Object.create in the scenario you describe.

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Jul 15, 2015
@dead-claudia
Copy link

And Object.create(Foo.prototype) is not necessarily the same as new Foo(). Especially if the constructor itself defines methods passed as arguments. And the other option is a matter of where the cast goes. Usually, the second is better.

let foo = Object.create<FooLike>(Foo.prototype);
let foo = <FooLike> Object.create(Foo.prototype);

@matthewjh
Copy link
Author

@IMPinball Of course it isn't, but I don't really see what relevance that has here. Also, I don't think the former line in your block of code is a cast, but a type parameter.

@RyanCavanaugh, here's an example:

interface IHouse {
   location: Coordindate;
   price: number;
   developerName: string;
}

var baseHouse = {
  location: new Coordinate(50, 100),
  price: 350000,
  developerName: 'Test Developer'
};

var house1 = Object.create(baseHouse);
house1.price = 400000;

var house2 = Object.create(baseHouse);
house2.price = 360000;

// Assume the type of buildHouses is `(houses: IHouse[]) => void`
buildHouses([
   house1,
   house2
]);

This code will work because house1 and house2 will be typed as any via Object.create, even though we know that its return value will have type typeof baseHouse. Why not add type information where possible so that the type checker has a greater scope to be useful… isn't that a 'tenet' of TypeScript? For me it's not so much a matter of 'do we need typing here?' but more one of 'why are we using 'any' in a case where the return type is entirely determined?'

@kitsonk
Copy link
Contributor

kitsonk commented Jul 18, 2015

@matthewjh I understand what you are saying, and from an overall logic perspective it makes sense, but what problem do you see with this?

var house1 = <IHouse> Object.create(baseHouse);
house1.price = 400000;

var house2 = <IHouse> Object.create(baseHouse);
house2.price = 360000;

If you made the change you suggested, everyone using Object.create would have to pass an type argument, breaking a lot of peoples code, making it more difficult for people to migrate to TypeScript, when in fact to get the same effective type guarding you have to type the same amount of letters, just a little bit to the left.

@matthewjh
Copy link
Author

That's a fair point, but the same could be said for many things:

var doubledNumbers = numbers.map((x) => x * 2);
vs
var doubledNumbers = <number[]> numbers.map((x) => x * 2);

Sure, we can use a cast based on what 'we' think the type is, but we shouldn't wherever possible. That's why we have a type inference system and type checking in the first place -- to cut humans out of figuring out types themselves and thereby making errors.

The problem with casting the return value of Object.create is the same as the problem with casting in general: we're telling the type system that value a WILL have type T, and that it should accept that as fact.

Given

var house2 = <IHouse> Object.create(baseHouse);

what happens if baseHouse is changed so that it no longer implements IHouse? This line of code doesn't break as far as the TypeScript compiler is concerned, and if we try to use house2 as an IHouse it of course passes type checking happily -- but it shouldn't. I believe this would likely cause a runtime rather than compile time error, which is exactly what static type analysis systems are supposed to prevent.

If the definition of Object.create was

create<T>(o: T): T;

this would be easily prevented and picked up during type analysis without introducing any breaking changes (because T could be inferred).

The problem is that when property descriptors are passed to Object.create the returned type will be a supertype of T, which is why my original proposal was
create<T extends A>(o: A, properties?: PropertyDescriptorMap): T;

However, that would be a breaking change as you point out (assuming TypeScript's type inference system isn't able to infer cases where typeof A === typeof T, and reduce T extends A to T, i.e. the case where no new properties are added via descriptors... I have no idea). Is there any reason why the definition below wouldn't suffice WITHOUT the need for breaking changes?

create<T>(o: T, properties?: PropertyDescriptorMap): T;

T could still be inferred in most (all?) cases, and the typing will still, I believe, be correct even if property descriptors are passed, because if you add properties to a T it's still a T. If you want to assign to a supertype with additional properties that are added via descriptors, you would have to cast it, but I would suggest that this is a very, very, very, rare use case.

I'm still new to TypeScript myself, so feel free to point out any errors I've made. :)

(By the way, Kit, I recognised your name in the notification of your comment. I used to work at Sky!)

@dead-claudia
Copy link

Okay. I see what you mean WRT interfaces. Object.create doesn't change interfaces, but it doesn't work with class prototypes.

@dead-claudia
Copy link

And as for Object.create<T>(object: T), that could be done as a PR now, without any extra implementation work.

@dead-claudia
Copy link

Oops... Ignore that comment. (that would require using only the interface, which TypeScript doesn't support.)

@mhegazy mhegazy closed this as completed Aug 10, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

5 participants