microsoft / TypeScript

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

RFC: Support __proto__ literal in object initializers #38385

Open mike-marcacci opened 4 years ago

mike-marcacci commented 4 years ago

This is a new issue to specifically propose and elaborate on a feature I raised in this comment on #30587.

Search Terms

Suggestion

While accessing or mutating an existing object via the Object.protptype.__proto__ getter/setter is deprecated, to the best of my knowledge defining the prototype of a new object via the object initializer __proto__ literal is very much encouraged.

The rules for specifying the prototype of a new object via these semantics are very well-specified and safe. See the relevant section of the spec here.

Basic support

Given the following:

const foo = {
  __proto__: { a: "a" },
  b: "b"
};

Typescript currently thinks that foo is the following shape:

type foo = {
  ["__proto__"]: { a: string };
  b: string;
};

when in reality, it is:

type foo = {
  a: string;
  b: string;
};

TypeScript should be able to correctly detect the type of this object initialization.

Strict validity checks

Additionally, TypeScript should prevent invalid __proto__ assignments that are "ignored" by the spec, and require all values to be null or an object. This should fail validation:

const invalid = { __proto__: "hello" }

Correct handling of computed properties

It's important to note that per the spec, __proto__ literals are not the same as regular property assignments.

This object initialization, for example:

const foo = {
  __proto__: { a: "a" },
  b: "b",
  ["__proto__"]: { c: "c"},
};

creates an object of the following shape:

type foo = {
  a: string;
  b: string;
  ["__proto__"]: { c: string};
};

Given this, I would recommend that a __proto__ literal be forbidden in type/interface definitions, such that this is considered a syntax error:

type foo = {
  __proto__: { a: string }
}

while this is an allowable way to specify a property named __proto__ on the type foo.

type foo = {
  ["__proto__"]: string
}

Use-Cases & Examples

This feature allows TypeScript to correctly understand the shape of objects defined with standard JS semantics. While this pattern isn't especially prevalent, it is an important feature of the language, and should be much more common in one particular use-case where TypeScript currently has a rather severe blind spot:

TypeScript currently PREVENTS the creation of safe indexed objects derived from existing indexed objects. For example:

Given this object, and the goal of "spreading" it into a new map:

// All safe map objects MUST have a `null` prototype.
const someMapObject: { [key: string]: boolean } = Object.create(null);

The following is UNSAFE, and probably the most common approach I see people using. TypeScript should catch this, and should issue a compile-time error. See bug #37963.

const unsafeSpreadMapObject: { [key: string]: boolean | undefined } = {
  ...someMapObject,
  foo: false
};

console.log(typeof unsafeSpreadMapObject["constructor"]);
// => function

console.log(typeof safeSpreadMapObject["foo"]);
// => boolean

The following is also UNSAFE. While using Object.assign and Object.create is a perfectly valid alternative, the any returned by Object.create propagates through the statement and breaks type safety. (Perhaps the result of Object.create should be unknown instead of any?)

const unsafeAssignMapObject: { [key: string]: boolean | undefined  } = Object.assign(
  Object.create(null),
  { foo: "this is not boolean" }
);

console.log(typeof safeSpreadMapObject["constructor"]);
// => undefined

console.log(typeof safeSpreadMapObject["foo"]);
// => string

This is the SAFE way to accomplish this while using object spreads, but TypeScript currently forbids it, since it lacks support for the __proto__ literal, and incorrectly believes a property of type null is being defined:

const safeSpreadMapObject: { [key: string]: boolean | undefined } = {
  __proto__: null,
  ...someMapObject,
  foo: false
};

console.log(typeof safeSpreadMapObject["constructor"]);
// => undefined

console.log(typeof safeSpreadMapObject["foo"]);
// => boolean

Checklist

My suggestion meets these guidelines:

References

dhoulb commented 4 years ago

Just ran across this and was surprised __proto__ in initialisers wasn't already supported.

{ __proto__: myPrototype } is much cleaner than Object.create(myPrototype), especially when you want to merge in additional properties at the same time.

I acknowledge the confusion that __proto__ in initialisers is pretty clear, but obj.__proto__ is confusing and should probably be discouraged, but I'd still love to see it supported.

hahanein commented 4 years ago

Also just ran accross this issue.

FWIW: According to @rauschma [0]:

I recommend to avoid the pseudo-property proto: As we will see later, not all objects have it.

However, proto in object literals is different. There, it is a built-in feature and always available.

[...]

  • The best way to set a prototype is when creating an object – via proto in an object literal or via:
Object.create(proto: Object) : Object 
ExE-Boss commented 4 years ago

Also, the __proto__ initialiser in object literals is in the process of being moved out of Annex B: https://github.com/tc39/ecma262/pull/2125.

Jack-Works commented 3 years ago

supported in PR #42359, behavior:

__proto__ for a non null | object type is a compile error. {...Q, __proto__: T, ...W} is treated as {...T, ...Q, ...W} at the type level.

Josh-Cena commented 2 years ago

@sandersn asked us to post thoughts in the issue (from #48816), so I'm going to throw in my 2c.

My idea is:

It's standard ECMAScript syntax, so even if we don't bring in the whole prototypical semantics, it should at least be special-cased to not mess with valid things like const map: Record<string, string> = { __proto__: null }

I see several levels at which TypeScript as a language can support __proto__.

  1. (Most ideal) It should have the syntax and corresponding semantics to describe the entire prototype chain. This is related to https://github.com/microsoft/TypeScript/issues/44253 (although I'm surprised no-one brought it up there): Object.hasOwn would have slightly different semantics from "key" in obj, especially if TS wants to support narrowing class instances.
  2. (1) is obviously very hard to achieve given TypeScript's current architecture, so taking one step back, I think #48816 is a good compromise, where it not only recognizes __proto__, but also gives its semantics enough love. (From my quick scanning, it types { __proto: p, ...obj } as typeof obj & typeof p.) Note, however, that such compromise is ultimately incorrect, and may hinder us from doing future meaningful work around prototype semantics.
  3. At the very, very minimum, TypeScript should ignore __proto__ (requested in https://github.com/microsoft/TypeScript/issues/13933), because after all, this is a special ECMAScript syntax, and is not totally obscure. Imagine TS thinking that super in object literals is just a random identifier. Even today's world where it's typed as any is better than that. Using { __proto__: null } to make a map is still common practice, and it should not prevent the object from being typed as Record<string, string>.

I simply don't think "we should not think about it" is an adequate way to approach this, especially when there's a PR in place. I understand it's a difficult decision to make, and I would personally go with approach (2) (as it's currently implemented), but be noted about its potential footgun. Expressing class inheritance has the same footgun in TS today, after all.

bakkot commented 2 years ago

Very concretely: { __proto__: null } for a dictionary isn't just common, it's also good; having a dictionary without __proto__: null is just incorrect. TypeScript considers let dict: Record<string, string> = { __proto__: null } to be a type error, which discourages the right thing and encourages the wrong thing. That's bad. And fixing this particular thing doesn't require getting the full semantics of prototypes into the type system; any of the three options mentioned in the previous comment would fix it, and would be a strict improvement on the current very-incorrect semantics.

As mentioned above, __proto__ in object literals is a standard part of ECMAScript. It is not deprecated or restricted to strict-only or any of that. TypeScript should not neglect it.

Josh-Cena commented 2 years ago

I was hesitant to call it "good", because if you actually want to work seriously with maps, you probably should use an actual Map anyways...

bakkot commented 2 years ago

Nah, { __proto__: null } is a perfectly fine way to work with a string-keyed map.

ExE-Boss commented 2 years ago

TypeScript considers let dict: Record<string, string> = { __proto__: null } to be a type error, which discourages the right thing and encourages the wrong thing.

I currently do:

let dict: Record<string, string> = {
    // the non-null assertion causes this to become `never`,
    // which is assignable to a `string`:
    __proto__: null!,
};

The other option is to use as any.

aduh95 commented 11 months ago

Another place where this issue is a pain:

Object.defineProperty({}, 'someKey', {
  __proto__: null,
  configurable: true,
});

This perfectly valid JS code is flagged as invalid by TypeScript (https://www.typescriptlang.org/play?#code/PIIwVgpgxgLgdAEwgMwJYDsIAUBOB7ABwhxgE8AKAbwF8AaAAgHIBnPAWwgGkJTGHKAUPXoB9EQXww8YgFz10AVwA2S2kPpQ86NAHMFOAIYglEOTBwKIa6gEoA3AKA).

dmchurch commented 7 months ago

Since this is still marked as "Awaiting More Feedback", I'll chime in with the chorus to say that this is seriously hindering my development efforts. Can we please get an update from Microsoft on this 4-year-old issue? At the very least, @RyanCavanaugh can you explain what further feedback is needed, or remove the tag?

Josh-Cena commented 7 months ago

If this puts more weight on decision making: as MDN's JS docs writer, in recent years I've added a lot more documentation about __proto__, highlighting its appropriateness in object initializers. It is the very first syntax introduced in the Inheritance and the prototype chain guide, even before new and Class.prototype. It is also used a lot more than Object.create throughout our code examples. My hope is that it gets preferred over Object.create in most use cases, especially if there are other known keys, because it's much more statically analyzable.

ljharb commented 7 months ago

It’s also safer, in that Object.create can be tampered with, but literal proto syntax can’t be.

mike-marcacci commented 6 months ago

@RyanCavanaugh and team I hate to be a nag, but is there anything in particular about this that is still "awaiting more feedback" as the label suggests?

This is a pretty straightforward suggestion that simply brings TypeScript in line with correct ECMA semantics, and there have been no concrete objections in the 4 years it's been open.

This continues to promote unsafe programming patterns (creating map objects without __proto__: null) that appear in TypeScript tutorials all across the web, and many of our internal codebases are speckled with "TODO" comments explaining some workaround and linking to this issue.

mkermani144 commented 3 months ago

I'm using such wrappers to bypass the limitation:

const withProto = <Proto extends object, Base extends object>(
  proto: Proto,
  base: Base,
) =>
  ({
    __proto__: proto,
    ...base,
  }) as unknown as Proto & Base;

Clearly a pain point.