[css-typed-om] Do CSSMathValue subclasses copy their constructor arguments?

[darrnshn]

When you pass CSSNumericValues to construct a subclass of CSSMathValue, do the arguments get copied? For example,

let x = CSS.px(1);
let y = CSS.px(1);
let sum = CSSMathSum(x, y);
// Q: are sum.values[0] and x the same object?
x.unit = 's'; // Should this be an error since now sum is adding a length with time?

I couldn't really find any wording for this; sorry if this was already implicit in the spec.

Spec: https://drafts.css-houdini.org/css-typed-om-1/#dom-cssmathsum-cssmathsum

[nainar]

Transcribing conversation with Tab.

Q: are sum.values[0] and x the same object?
Yes. The first step says, "Replace each item of args with the result of rectifying a numberish value for the item." In this case the item would be equal to x.

[nainar]

Marking the second question as Needs-Feedback.

[darrnshn]

I asked Shane and it looks like when you're mutating an expression tree, it should propagate type checking up. So setting x.unit = 's' should be an error. I couldn't find anything in the spec that explicit states this though; maybe we should add a note?

[css-meeting-bot]

The Working Group just discussed Do CSSMathValue subclasses copy their constructor arguments?, and agreed to the following resolutions:

• RESOLVED: Change type to be immutable

The full IRC log of that discussion

<nainar> Topic: Do CSSMathValue subclasses copy their constructor arguments?
<TabAtkins> GitHub: https://github.com//issues/494
<nainar> TabAtkins: When I first drafted math classes they converted to new objects when read out. People didnt like that.
<nainar> TabAtkins: Algo assumptions depended on getting fresh valudes that werent modifiable. So a px and a s would throw. But you can change the unit later. G9oes undetected until later.
<nainar> TabAtkins: We cant do checks at mutation time. object moght be used in multiple places, where some might be fun but not all.
<nainar> TabAtkins: So proposal is to restrict changing the type of MathVaues. Stay within category at least.
<nainar> TabAtkins: Dont even think there is need to change within category. Happy to make type readonlhy
<nainar> TabAtkins: Impact - cant change 5 px to 5 em by changing type. Do by constructing a new value.
<nainar> TabAtkins: Minimal addition API weight for someone wanting to do this. (change px to em)
<nainar> Rossen: You can easily wrap what you just said
<nainar> TabAtkins: to method already does that
<nainar> TabAtkins: Objections to changing type to read only?
<dbaron> I'm generally happy with immutability.
<nainar> RESOLVED: Change type to be immutable

[tabatkins]

So the resolution doesn't solve this problem. It prevents you from changing the overall type of something if it's only composed of CSSUnitValues, but not if it's composed of CSSMathValues. That is, this is still allowed:

let child = new CSSMathProduct(CSS.px(5));
let parent = new CSSMathSum(child, CSS.px(10));
// parent == calc(calc(5px) + 10px), everything's good
child.values[0] = CSS.s(5)
// parent == calc(calc(5s) + 10px), invalid!

Calling child.mul(CSS.s(5)) doesn't cause this problem, because the math ops always return a fresh object; it wouldn't mutate what's in parent.

So, I think we need to have a similar lock-down on setting .value. Similar to .type, we can either make it readonly (which still lets you mutate the numeric value inside of there), or just limit it to things of the same type.