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

feat: support __proto__ in object literal (type-check only) #42359

Closed
wants to merge 1 commit into from

Conversation

Jack-Works
Copy link
Contributor

@Jack-Works Jack-Works commented Jan 15, 2021

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Jan 15, 2021
@typescript-bot
Copy link
Collaborator

The TypeScript team hasn't accepted the linked issue #38385. If you can get it accepted, this PR will have a better chance of being reviewed.

@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

Comment on lines +63 to +64
var x6 = { __proto__: __proto__ };
var x7 = { __proto__: function () { } };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two have to be transpiled as:

var x6 = {};
Object.defineProperty(x6, "__proto__", { value: __proto__, configurable: true, enumerable: true, writable: true });
var x7 = {};
Object.defineProperty(x7, "__proto__", { value: function () { }, configurable: true, enumerable: true, writable: true });

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does ES5 support { ['__proto__']: val}?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, it doesn’t.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, not lucky

Comment on lines +66 to +68
(function (e) {
e[e["__proto__"] = 1] = "__proto__";
})(e || (e = {}));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incorrect because of the legacy __proto__ accessor, so it has to use Object.defineProperty.

Comment on lines +69 to +71
{
var __proto__1 = (_b = {}, _b['__proto__'] = 1, _b).__proto__;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here for the definition of _b.

Comment on lines +55 to +56
var z1 = __assign(__assign({}, ({ a: '' })), { __proto__: o });
var z2 = __assign({ __proto__: o }, ({ a: '' }));
Copy link
Contributor

@ExE-Boss ExE-Boss Jan 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The __proto__ property initialiser is supposed to set the [[Prototype]] of the first object, so this has to be:

var z1 = Object.setPrototypeOf(__assign({}, ({ a: '' })), o);
var z2 = __assign({ __proto__: o }, ({ a: '' }));

Although engines most likely do something like:

var z1 = __assign({ __proto__: o }, ({ a: '' }));
var z2 = __assign({ __proto__: o }, ({ a: '' }));

for both.

@@ -38,7 +38,7 @@ var o = {
};
var b = o["__proto__"];
var o1 = {
__proto__: 0
__proto__: {}
};
var b1 = o1["__proto__"];
Copy link
Contributor

@ExE-Boss ExE-Boss Jan 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accessing the __proto__ property like this should be a compile error, unless __proto__ is defined on the global Object interface or the __proto__ property is declared using { ["__proto__"]: ... }.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, as you can see some of my changes reflected that:

tests/cases/conformance/es6/modules/m2.ts(2,21): error TS2339: Property '__proto__' does not exist on type '{ __esmodule: boolean; }'.

@Jack-Works
Copy link
Contributor Author

Now I only change the type checker around __proto__ I'll revisit @ExE-Boss's review on emitting __proto__ later, maybe some suggestion from the TypeScript team members

@sandersn
Copy link
Member

I don't understand the spec process for moving things out of Appendix B. Is there a Stage 1,2,3,4 process? Or does it just need an ad-hoc OK from the committee? Has the __proto__ syntax move finished the process yet?

As long as __proto__ is in appendix B with all the other "icky" stuff, I don't think it's worthwhile supporting it in Typescript.

@Jack-Works
Copy link
Contributor Author

Jack-Works commented Feb 19, 2021

@sandersn This PR is about the proto literal syntax which is not normative optional.
The optional one is Object.prototype.__proto__ and they're different

@rwaldron
Copy link

@sandersn re:

As long as proto is in appendix B with all the other "icky" stuff, I don't think it's worthwhile supporting it in Typescript.

If Typescript is intended to be used for authoring code that targets the web, then Annex B support is required:

The ECMAScript language syntax and semantics defined in this annex are required when the ECMAScript host is a web browser. The content of this annex is normative but optional if the ECMAScript host is not a web browser.

One could argue that the either of those sentences is true, but also that they are false. I think it's reasonable to err on the side of flexibility and go with "Typescript should either act like, or make it possible to configure as, the web"

@Jack-Works Jack-Works changed the title feat: support __proto__ in object literal feat: support __proto__ in object literal (type-check only) Jun 24, 2021
@Jamesernator
Copy link

The object literal form of __proto__ moved out of Annex B in July, so this syntax is canonical and required by the spec. The behaviour of __proto__ is now just a regular part of the definition of parsing object literals.

The Object.prototype.__proto__ accessor remains normative optional, and I don't believe there's any interest of changing that.

@Jack-Works
Copy link
Contributor Author

👀 This PR has been 13 months, is there any progression?

@Jack-Works
Copy link
Contributor Author

Jack-Works commented Apr 23, 2022

I changed the branch name of this PR from "master" to "proto-literal-type-checking" and GitHub closes this PR. I opened a new PR, please follow #48816.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Ignore __proto__
7 participants