Should we allow the wrapper object to have additional symbol properties

[rricard]

While writing spec text we found out that we could either allow writing new arbitrary props to the exotic record boxing object or simply disallow them. The champion group would like to do the latter but if there are objections, we're interested to have them here.

Here is an example that we would want to disallow:

var o = Object(#{});
o.foo = true;
o.foo // => true

The main rationale against disallowing this would be that this is acceptable with a String for instance:

var o = Object("a");
o.foo = true;
o.foo // => true

This should be decided before Stage 3.

cc. @ljharb

[ljharb]

Why would you prefer to disallow it?

Primitives can be as exotic as desired, but it seems preferable to minimize the ways in which objects - even boxed primitives - are exotic.

[ljharb]

Additionally, if I can't set Symbols on a boxed Record object, then I can't opt them into any protocols, which is pretty important.

[rricard]

To me it's in order to avoid any mistake of having a boxed record and starting to see I can mutate it. I don't see many people will wrap their Record into their boxing objects though and passing them so honestly I'm not against going back and changing this. I'd still like to wait next week to hear @littledan's opinion on this however.

[rricard]

Additionally, if I can't set Symbols on a boxed Record object, then I can't opt them into any protocols, which is pretty important.

That might do it to reverse that decision, still invoking @rickbutton for a second champion opinion

[rickbutton]

I don't really have a strong opinion on this either way. I can see the argument against allowing adding string properties to a boxed Record, for the same reason that boxed Records have a null prototype - that we don't want to allow Records to surface string-properties that don't come from the [[RecordData]] itself. However, attaching symbols to boxed records seems like a reasonable desire to me.

There was some previous discussion about these ideas in this thread: https://github.com/tc39/proposal-record-tuple/issues/71#issuecomment-633894383 (although, not for new-properties on a boxed Record, but instead for the prototype).

[littledan]

Personally, I'm pretty opposed to permitting additional properties on Record wrappers. The domain of Records is their data properties, and it would be rather broken to have to think about whether the property is on the Record or its wrapper. Being frozen is not very exotic, and it corresponds to the normal mental model around Records and Tuples, so it's unsurprising.

@ljharb Could you explain the part about presence in protocols? It's very hard for me to understand how this would be useful, given that the underlying Record will not have the symbol properties, and the Record wrapper will not act like Records in key ways (e.g., === is broken).

[ljharb]

That's a fair point that it'd be awkward to make a wrapper object just to install Symbols, and I just realized, and filed #142, about the seeming current inability to extract a primitive Record from a boxed Record object - which would be strictly necessary, both for "boxing the primitive for the purposes of protocol participation, and in general. I think it might make sense to hold off further discussion on this issue until that's resolved, since I'm somewhat convinced that the only outcome of it is Record.prototype, which would require exoticness so that string properties on boxed objects weren't possible, and then adding custom Symbols to a boxed object or to Record.prototype would seemingly Just Work.

[littledan]

and then adding custom Symbols to a boxed object or to Record.prototype would seemingly Just Work.

I think this comment is mixing together two different things. One question is whether Record and Tuple wrappers are frozen, and another thing is whether we defer to Record.prototype on access of Symbol-keyed properties. Mechanically, they're unrelated. I think it's important that Record and Tuple wrappers are frozen, but I'm open to Record property access deferring to Record.prototype for Symbol properties.

[ljharb]

If Record.prototype is a mutable object, then the use cases for mutating an individual wrapper are certainly fewer - but i don't see why these should be the only primitive wrapper objects that are frozen, and there still are use cases where I'd want to opt a wrapper into a protocol without opting in every wrapper object.

[rickbutton]

It is still possible to opt-in an immutable wrapped Record into a protocol with a Proxy.

[ljharb]

No, it’s not, because the Proxy wouldn’t have the internal slots of a Record, so unboxing would fail.

[littledan]

That's true, it wouldn't have the internal slots of a Record. But a Record wrapper that you could add other properties to wouldn't act like a Record, e.g., with ===. Overall, I'm still having trouble understanding when you'd want to use this construct.

[ljharb]

Right, but wrapped[Symbol.toPrimitive]() is a cheap call, and would work with ===, while wrapped could still contain extra data via regular properties.

[littledan]

Could you give an example of how this would use in practice? I can imagine a program using this corner of the language, but I can't understand why anyone would want to write it.

If you're going to explicitly unwrap it for some operations, it's unclear to me what the problem is with Proxy-wrapping.

[ljharb]

You can unwrap a boxed primitive; you can't unwrap a Proxy around one (because Record.prototype[Symbol.toPrimitive].call(proxy) would always throw.

I don't have a concrete example to offer - i'm coming from a position of maximizing composability and consistency with the way primitives and objects and internal slots already work. I'm also thinking vaguely in terms of what tricks might be employed by a R&T polyfill, and I don't want to discover too-late that something that I'd expect to work, doesn't :-)

[littledan]

Yes, I can see how the difference is observable. I'd like to understand more what you'd like to compose this with. I agree that it's important to follow typical patterns in the language, and I think "behaving like a frozen object" achieves that. I don't think Records and Tuples need to slot into all existing cases which expect mutable, extensible objects, since the whole point of Records and Tuples is to be deeply immutable, but I'm open to being convinced with examples until Stage 3.

[littledan]

I remain confused by the arguments above for allowing additional properties on Box wrappers. I think we should conclude that Box wrappers are also prevent-extensions, like Record and Tuple wrappers.

Edit: Oh, wow, I completely misunderstood this thread when re-skimming it. Anyway, I still agree with my previous comments above 😇 and would prefer to settle this issue on the frozen-wrappers side. Note that if we move to identityless objects, the question is moot--they must be frozen.

[mhofman]

In the context of R&T as identity-less objects, as @littledan mentions the question would be moot. There would be no object wrapper, the R/T would be the object, which would be frozen and prevent any extension.

In the context of identity-less objects I was suggesting that proxies of R&T would behave like regular objects. However I don't think that would solve @ljharb's use case with adding properties to a wrapper of R/T, since with a frozen target, all properties would need to exist on the shadow target. However I'm still not sure I understand the motivation to extend a R/T that way.

[nicolo-ribaudo]

After implementing R&T in SpiderMonkey, I'm in favor of letting the object wrappers to be mutable (as they are for other primitives).

There are many places where the spec calls ToObject on a generic ECMAScript value, and this means that object wrappers are frequent. They can be optimized away in hot paths, but optimizing them away everywhere introduces a complexity that might not be worth the performance improvement.

When implementing R&T wrappers, I used the same approach already used for string wrappers (since strings are the only existing primitive with own properties): Object(tuple) returns an empty object, and its properties are lazily defined only when you try to observe them.

For example, this is how the wrapper object internally transitions between the different shapes:

let o = Object(#{ x: 1, y: 2, z: 3 }); // o is {}
o.x; // o is { x: 1 }
o.w; // o is { x: 1 }
delete o.y; // o is { x: 1, y: 2 }

This is not observable from JS since properties appear as soon as you try to observe them, but gives a big advantage by making Object(record) an O(1) operation rather than an O(n) operation, since it doesn't require to eagerly copy all the properties.

If object wrappers were not extensible, this would be harder to accomplish. It's possible to internally define new properties and skip the "is it extensible?" check, but there are optimizations for non-extensible objects that would stop being safe (since you cannot rely anymore on the non-extensibility to be sure that it won't get new properties).

[mhofman]

What would the following do:

let o = Object(#{ x: 1, y: 2, z: 3 });
Object.defineProperty(o, 'w', {value: 0, enumerable: true});

If modeled like other object wrappers it'd be allowed, right?

[nicolo-ribaudo]

Yes, I'm proposing that it should work.

[ljharb]

It’s an object; it’d be strange if it’s not allowed.

[mhofman]

How do we ensure the following:

const r = #{ x: 1 };
Object(r).y = 2;
assert.throws(() => r.y = 2);

I know that strings behaves that way, but I'm unclear what in the spec confers that behavior.

[nicolo-ribaudo]

I don't understand exactly why it works for strings, but I plan to copy the same behavior.

[gibson042]

In what sense does it "work" for strings?

$ eshost -sx 'const s = "foo", p = 1; Object(s)[p] = "x"; s[p] = "x"; print("no error")'
#### ChakraCore, engine262, GraalJS, Hermes, JavaScriptCore, Moddable XS, SpiderMonkey, V8
no error

[mhofman]

Sorry I should have said strict mode is important:

$ eshost -sx '(function() {"use strict"; const s = "foo", p = 3; Object(s)[p] = "x"; s[p] = "x"; print("no error")})()'
#### ChakraCore

TypeError: Assignment to read-only properties is not allowed in strict mode

#### engine262

TypeError: Cannot set property '3' on 'foo'

#### Hermes, V8

TypeError: Cannot create property '3' on string 'foo'

#### JavaScriptCore

TypeError: Attempted to assign to readonly property.

#### Moddable XS

TypeError: ?: set 0: not extensible

#### SpiderMonkey

TypeError: can't assign to property 3 on "foo": not an object

I would have assumed String Exotic object [[DefineOwnProperty]] had a check for the this value being the string primitive or an object wrapper, but that doesn't seem to be it, so I'm not sure.

Edit: I assumed wrong and the check is done earlier in OrdinarySetWithOwnDescriptor step 2.b. If Type(Receiver) is not Object, return false.

[ljharb]

I think it might be in the actual assignment semantics, but I’m not certain. Either way, I’d assume assigning to any non-nullish primitive behaved identically as any other, in sloppy mode and strict mode.

[mhofman]

@nicolo-ribaudo could you clarify your implementation feedback?

The current spec text doesn't seem to allow #{ x: 1 }.y = 2 whether in strict mode or not, neither does it allow Object.defineProperty on a record wrapper object (unless same description of course).

Why would [[Set]] and [[DefineOwnProperty]] operations that allow those be better for implementations? Like proxy objects that lazily fetch their properties on access, you should be able to lazily surface the properties of the record wrappers while disallowing the actual operations that modifies the properties?

@ljharb while I agree it'd be more consistent with other primitives, I'm concerned the [[Set]] semantics in non-strict mode are a foot gun for this new primitive type (we shouldn't be bound to sloppy compatibility on this). And it'd probably be weird to disallow [[Set]] but to allow [[DefineOwnProperty]], but that's more debatable. Also, I have even less of an opinion whether the restriction should only apply to string properties, or to symbol properties as well.

[ljharb]

To be fair, i wouldn't object to a new primitive behaving strictly even in sloppy mode, since that seems in keeping with the spirit of strict mode - altho that might break code that assumes such assignment will never throw.

[mhofman]

Arguably the following is still a sloppy mode footgun:

const r = #{ x: 1 };
const s = Symbol();
r[s] = 'foo';

So while having [[Set]] disallow any assignment but allow [[DefineOwnProperty]] to define symbols might be weird, it probably is the only sensible approach if you want to allow symbols on wrapper objects. I still find it questionable one would want to defineOwnProperty of strings properties on record wrapper though.