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

Will __proto__ be treated as prototype for records? #46

Closed
tolmasky opened this issue Sep 3, 2019 · 27 comments · Fixed by #99
Closed

Will __proto__ be treated as prototype for records? #46

tolmasky opened this issue Sep 3, 2019 · 27 comments · Fixed by #99

Comments

@tolmasky
Copy link

tolmasky commented Sep 3, 2019

In the following:

const x = #{ __proto__: #{ y: 10 } }

Is the expectation that x.y === 10?

@mheiber
Copy link
Contributor

mheiber commented Sep 3, 2019

I don't think Records have a proto. They don't participate in prototype chains.

@ljharb
Copy link
Member

ljharb commented Sep 3, 2019

Typically every primitive that's dotted or bracketed into creates a boxed object, on which __proto__ is accessed. This proposal contains an entirely new kind of primitive that changes the meaning of . and [], so assuming that's acceptable, I'd assume that .__proto__ and ['__proto__'] can have different meanings, including "normal data properties".

@mheiber
Copy link
Contributor

mheiber commented Sep 4, 2019

@ljharb do you think there is a way to achieve all three of the following?

  1. not changing the meaning of .
  2. dot access for Records
  3. deep immutability for Records

(similar questions apply for Tuple)

@ljharb
Copy link
Member

ljharb commented Sep 4, 2019

@mheiber number 1 in that list would mean that these new types must be objects, not primitives, since dot/bracket access always either throws (nullish), or boxes into an object and returns the resulting property (primitive), or returns the resulting property (object).

@mheiber
Copy link
Contributor

mheiber commented Sep 4, 2019

Sounds like there's no way, then. Anything with Object in its prototype chain is mutable, as far as I know.

@ljharb
Copy link
Member

ljharb commented Sep 4, 2019

It’s not about mutability; it could have a [[Prototype]] of null like Object.create(null) - or it could be a primitive that’s boxed into a Record object who has a prototype of Record.prototype which in turn has a [[Prototype]] of null - but for polyfill and security use cases, Record.prototype wouldn’t be able to be immutable.

@ljharb
Copy link
Member

ljharb commented Sep 4, 2019

iow i think the “bare member access” feature requires it be a proxy, a normal object, or a brand new kind of thing.

@littledan
Copy link
Member

littledan commented Sep 19, 2019

I think we have two alternatives here:

  1. Treat #{ __proto__: xyz } as #{ ["__proto__"]: xyz }, i.e., as a data property. It would not set the prototype (presumably, we're talking about the prototype of the result of ToObject).

  2. Make it an early error to have #{ __proto__: xyz }, forcing people to instead write #{ ["__proto__"]: xyz }. This would also have no effect on the prototype.

I can't make of my mind which one of these I like better.

@littledan
Copy link
Member

Does anyone have any thoughts on 1 vs 2 of #46 (comment) ? I guess I'm leaning towards 2. (I hope this is relatively easy thing to nail down; it's a bit of an edge case.)

@andre-matulionis-ifood
Copy link

I personally think it should be consistent, so 1 would be appropriate. Forcing it to set it as a computed property could confuse more than the alternative

I consider __proto__ as a special property, but still a property. The alternative 2 would set a new precedent, one that __proto__ is more than a property, since it has this special setting rule.

Anyway, this use case never comes up, so I don't consider this a huge deal.

@littledan
Copy link
Member

@andre-matulionis-ifood I agree that consistency should be the main goal here. But I'm confused; isn't __proto__ already a special property? If you have it in an object literal, it already does weird things.

@andre-matulionis-ifood
Copy link

andre-matulionis-ifood commented Jan 21, 2020

I meant to focus on the "property" aspect. @ljharb put it best:

I'd assume that .__proto__ and ['__proto__'] can have different meanings, including "normal data properties".

AFAIK, no property has different behavior whether it is set as prop: xyz or ['prop']: xyz. My opinion is that it is not acceptable to add this aspect specifically to __proto__.


Another alternative is that it would throw a error independently of setting through __proto__: xyz and ['__proto__']: xyz.

@senocular
Copy link

No property has different behavior whether it is set as prop: xyz or ['prop']: xyz.

@andre-matulionis-ifood __proto__ does in normal object initializers.

If propKey is the String value "__proto__" and if IsComputedPropertyKey(PropertyName) is false, 
then
   Let isProtoSetter be true.
Else,
   Let isProtoSetter be false.
...
If isProtoSetter is true, then
  If Type(propValue) is either Object or Null, then
    Return object.[[SetPrototypeOf]](propValue).

https://tc39.es/ecma262/#sec-__proto__-property-names-in-object-initializers

So

a = { __proto__: { x: 1 } }
Object.getPrototypeOf(a) === a.__proto__ // true
a.x // 1
b = { ["__proto__"]: { x: 1 } }
Object.getPrototypeOf(b) === b.__proto__ // false
b.x; // undefined

Neither 1 or 2 are necessarily consistent. 1 changes the behavior so this special case is ignored for records while 2 effectively keeps the special case but throws because its not allowed, yet still preserves the ability to set that key using a computed property name.

@ljharb
Copy link
Member

ljharb commented Jan 21, 2020

That was the thrust of my comment; since __proto__ already works that way in objects, I’d expect it to work identically in Records.

@andre-matulionis-ifood
Copy link

Thanks for the correction. I forgot to put AFAIK in my comment. Did not know about it.

So my opinion is not valid. I don't have any comments about these alternatives.

@littledan
Copy link
Member

OK, I think this conversation was still good to have on the record in case others are coming from a similar place as @andre-matulionis-ifood .

Can we conclude option 2 of #46 (comment) then, of banning __proto__ in Record literals?

@ljharb
Copy link
Member

ljharb commented Jan 22, 2020

I’m not sure those are the only two options; we could also make it set the [[Prototype]] just like normal objects do, since that’s not what makes it a Record.

@littledan
Copy link
Member

Does anyone want to advocate for that option? I'd prefer if we omitted that feature.

@ljharb
Copy link
Member

ljharb commented Jan 22, 2020

I can see value in creating a record with a null proto, just like when i create objects with one. Forcing a separate Object.setPrototypeOf would be unfortunate (and would records then have .__proto__ always be undefined, be the [[Prototype]], or always be a normal data property even if not provided? I assume options 1 and 2 would suggest the latter, but that seems much weirder to me)

@littledan
Copy link
Member

@ljharb

I can see value in creating a record with a null proto, just like when i create objects with one.

One possibility is that Records would always have a null prototype. Personally, I'm leaning towards that, even while Tuples have a non-null prototype for Array-like methods.

Forcing a separate Object.setPrototypeOf would be unfortunate

I don't think Object.setPrototypeOf would work on Records; it'd throw a TypeError unless you gave null.

(and would records then have .proto always be undefined, be the [[Prototype]], or always be a normal data property even if not provided? I assume options 1 and 2 would suggest the latter, but that seems much weirder to me)

It seems like we all agree that Records should be able to have a data property named "__proto__", e.g., with the syntax #{ ["__proto__"]: foo}, just like normal objects. This wouldn't affect the [[Prototype]], and it'd work in both options 1 and 2.

@rickbutton
Copy link
Member

I personally lean towards option 2 (__proto__ early error, "__proto__" is a normal property), because I don't think under the current proposal attempting to set the __proto__ of a record makes much sense. While the exotic Record object does have a prototype, record primitives do not (because they are not objects). When you create a record with #{ __proto__: xyz }, you are not creating the exotic Record object, and therefore are not creating anything that can have a prototype, so attempting to assign a prototype to the record doesn't make any sense IMO.

You could, in theory, allow record primitives to somehow hold some kind of reference to their intended prototype, so that when they are boxed into Record exotics something special happens, but that seems like very weird behavior not expressed in the other primitives, and seems hard to specify correctly.

Given that, __proto__ being an early error makes sense to me, and allows us to re-evaluate in the future if desired.

@Jack-Works
Copy link
Member

I'd like to treat __proto__ as a normal property, with no early errors. that's as strange as to make eval?.() as a syntax error.

@littledan
Copy link
Member

@Jack-Works Can you say more about where you disagree with the reasoning described above?

@Jack-Works
Copy link
Member

proto is a old special case on the object literal.
Record literal is a totally new thing.
proto a normal property key name in the record literal so it does not make sense to make it a syntax error just because people want to use it in Record literal. Do not do any specical rules for it and the developer will see that proto doesn't mean anything special on the Record.

(Same argument for throwing on eval?.())

@littledan
Copy link
Member

littledan commented Jul 21, 2020

Do not do any special rules for it and the developer will see that __proto__ doesn't mean anything special on the Record.

How should we establish whether this assertion about developer intuition is true? Earlier in this thread, people were wondering whether __proto__ would work. I don't know how we can arrive at the conclusion that it wouldn't be confusing. I actually could imagine having a prototype chain consisting just of records and tuples, as the original post alluded to, though I don't think that would be a very good idea.

I see the eval case as different: Direct and indirect eval both exist within JS + Annex B, and it's just based on context which one triggers. By contrast, in JS + Annex B always makes __proto__ work in object literals--it's not conditional based on context. The only conditionality is between different environments.

@Jack-Works
Copy link
Member

How should we establish whether this assertion about developer intuition is true

I'm not guessing that what I want to tell them this doesn't work. When a developer wants to do this, they will try it #{ __proto__: x } and found "Ah, proto doesn't work in the Records, now I learned that."

Using __proto__ an anti-pattern so let's just keep it in the objects. records are a new type so it doesn't have to get polluted by the legacy of Object.

__proto__ is in Annex B so the only Web needs to have that. Making it an early error in Records makes it spreads.

@LongTengDao
Copy link

LongTengDao commented Aug 9, 2021

In normal object literal, both __proto__: value and "__proto__": value means proto defination, and only es6 new syntax like ["__proto__"]: value or shorthand __proto__ means property defination.

So the current solution (__proto__ is forbidden, "__proto__" is property) is very strange...

I think there are three ways:

  1. __proto__: value and "__proto__": value are forbidden, ["__proto__"]: value and shorthand __proto__ are property definition like in normal object literal;
  2. or, better, since record has no proto, all writting style are valid property;
  3. or, best, currently, use 1; when people know it well in the future, remove the limit in a new proposal, use 2 (this behaviour will be backward compatible).

@ljharb @littledan @rickbutton

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants