css-houdini-drafts icon indicating copy to clipboard operation
css-houdini-drafts copied to clipboard

[css-typed-om] Switch CSSUnparsedValue and CSSTransformValue to having an arrayish member?

Open tabatkins opened this issue 6 years ago • 13 comments

Background: After continually running into walls with getting better WebIDL support for array-likes, I gave up and designed CSSTransformValue and CSSUnparsedValue to use the family of existing WebIDL features that make something vaguely array-like (indexed getter/setter/creator, iterator, magic length, etc).

(CSSNumericArray is similar, but on a close look, appears to be an evolutionary spandrel. I've filed #947 about that.)


In https://github.com/heycam/webidl/issues/796, @domenic is figuring out how to add array-likes to WebIDL. They've got two complementary approaches:

  1. Creating two Array subclasses (for user-mutatable and not) in WebIDL, and letting interfaces extends from them to get all the good behavior (magic .length, all the array methods, returning true from isArray(), etc)
  2. Creating an arraylike<Foo>; declaration, a la maplike<>;, that an interface can declare that'll do most of the good behavior.

I get the feeling that 1 is kinda better than 2, and should be preferred when possible. We might be able to change CSSUnparsedValue and CSSTransformValue to do this, having a member that's a typechecked array, once those are finished being invented.

@domenic, is this feeling correct? Or would it be okay to leave them as-is?

tabatkins avatar Sep 17 '19 09:09 tabatkins

Right, I think it'd be better if we just starting treating the two new Array subclasses as "the thing", and web developers start to have expectations about them. So, in my mind, (1) will generally be better and preferred going forward.

Now, whether it's worth the potential churn and compat pain of switching the API surface, is another question, that I would leave up to you as the editor. (2) would certainly make more sense for just preserving the API shape. Although, even then, I'm unsure how we would get the behavior of "inherits from Array.prototype and gets all those methods" given that the classes in question inherit from CSSStyleValue... so maybe (2) isn't applicable here either.

domenic avatar Oct 17 '19 22:10 domenic

(2), the arraylike<>; declaration, would need to be like maplike<>/setlike<>, in that they don't inherit from the corresponding "real" classes, but do automatically (via the WebIDL spec) install all the appropriate methods on themselves.

(Note that maplike<> could not have been done by inheriting from Map anyway; the fact that you can bypass any checks done by the subclass's set() by just using Map.prototype.set.call makes subclassing worthless.)

But anyway, yeah, I think giving them an array-subclass member would be just fine; if needed we could still give the parent class an iterator over its child array or something.

Need to talk with @lilles about what seems reasonable to do in our impl.

tabatkins avatar Oct 22 '19 21:10 tabatkins

(2), the arraylike<>; declaration, would need to be like maplike<>/setlike<>, in that they don't inherit from the corresponding "real" classes, but do automatically (via the WebIDL spec) install all the appropriate methods on themselves.

Oh, no, I was planning on having it just inherit (since unlike Map, that's possible). That seems like it'd give the best results for e.g. "arrays with an extra item() method", which is the main use case I was considering.

domenic avatar Oct 23 '19 18:10 domenic

What happens, then, when someone does a direct set of an indexed property thru some of the metaprogramming facilities?

(If the answer is "it shadows the internal indexed property, but doesn't mess with the internal value", that's fine. If the answer is "it bypasses the typechecks and such and directly fiddles with the internal values"n like happens with Map subclasses, then that's unusable.)

tabatkins avatar Oct 28 '19 19:10 tabatkins

I'm not sure what scenario you're referring to; could you explain with code?

domenic avatar Oct 28 '19 19:10 domenic

I'm not sure, or else I would have done so. ^_^ Is there an equivalent to Map.prototype.set(mapLike, 'key', someRandomValueThatDoesntTypecheck), but for array subclasses, that would similarly bypass the typechecks?

tabatkins avatar Oct 28 '19 19:10 tabatkins

Nah, all changes to indexed properties (including by array methods) invoke the internal [[Set]]/[[Get]] methods, which would be specified for the new classes to do type checks.

domenic avatar Oct 28 '19 19:10 domenic

Ah, kk then, good.

So is it still the plan of record, then, that I can't create an interface MyClass extends ParentClass { arraylike<Foo>; };, because the magic has to come via extending Array?

tabatkins avatar Oct 28 '19 19:10 tabatkins

Yeah, that's what I'm thinking at the moment.

domenic avatar Oct 28 '19 20:10 domenic

All right, so that does still make me a little sad, and I think we should provide an arraylike<> that does fake-Arrays like maplike<> does fake-Maps, precisely for that usecase of "class that needs to extend something else but wants to act like an Array", but I believe I can adapt our APIs here to use an extends Array child.

tabatkins avatar Oct 28 '19 20:10 tabatkins

The CSS Working Group just discussed using ObservableArray for the list-ish values in TypedOM, and agreed to the following:

  • RESOLVED: put the list in a "value" property, and add a forwarding for compat
The full IRC log of that discussion <TabAtkins> Topic: using ObservableArray for the list-ish values in TypedOM
<TabAtkins> github: https://github.com/w3c/css-houdini-drafts/issues/948
<fremy> ScribeNick: fremy
<fremy> TabAtkins: I tried to get better support for array-like classes in WebIDL
<fremy> TabAtkins: because we have three things that expose lists, and could benefit from this
<fremy> TabAtkins: but given I made no progress, I ended up using the legacy "getter/indexer" of WebIDL
<fremy> TabAtkins: but sadly that means that it's not an array
<fremy> TabAtkins: and you cannot use forEach etc on them
<fremy> TabAtkins: you have to convert them to an array first
<TabAtkins> https://heycam.github.io/webidl/#idl-observable-array
<fremy> TabAtkins: but domenic fixed this for me
<fremy> TabAtkins: and we have the observable array type
<fremy> TabAtkins: for the javascript user, it looks like an array
<fremy> TabAtkins: but it still triggers callbacks that allow us to type check things
<fremy> TabAtkins: so I'd like to switch to that
<fremy> TabAtkins: unfortunately that would result in a breaking change
<fremy> TabAtkins: indeed, the value itself cannot be an array
<fremy> TabAtkins: because it has to subclass the CSSValue type
<fremy> TabAtkins: so we need to store the list in a "values" member
<AmeliaBR> q+
<fremy> TabAtkins: for the math-value types, it's easier because it's already a FrozenArray
<fremy> TabAtkins: so I can just switch it
<fremy> TabAtkins: any objection to do this change?
<fremy> AmeliaBR: so, if I understand, you now need to do "x.values[0]" instead of "x[0]"?
<fremy> TabAtkins: if I don't accomodate that by making a special behavor, yes
<fremy> AmeliaBR: could we then make a putforward?
<fremy> TabAtkins: yes, we definitely can
<fremy> q+
<fremy> TabAtkins: and we could do that by just updating the code to do that and lookup in the array
<fremy> AmeliaBR: browser support?
<fremy> TabAtkins: yes, typedom is shipped in chrome
<fremy> TabAtkins: because paint api
<fremy> AmeliaBR: I would not like to break demos
<fremy> TabAtkins: yes, let's make the accomodation, it's not worth breaking the demos
<fremy> AmeliaBR: (restates)
<fremy> TabAtkins: (yes)
<AmeliaBR> ack AmeliaBR
<Rossen___> ack AmeliaBR
<Rossen___> ack fremy
<fantasai> fremy: I'm entirely in favor
<fantasai> fremy: Always thought it was really weird that ? and ?? you couldn't index it
<fantasai> fremy: so much cleaner for the PAI
<fantasai> fremy: not only improvement that you get an array
<fantasai> fremy: definitely in favor
<fantasai> fremy: and ??? sounds like a no-brainer
<fremy> TabAtkins: ok
<bkardell_> s/PAI/API ?
<fremy> s/??/forwarding
<fremy> TabAtkins: any concern from implementers?
<fremy> TabAtkins: looks like no?
<astearns> s/???/forwarding
<fremy> TabAtkins: any objection to adopt the change in the issue, with forwarding kept in?
<fremy> RESOLVED: put the list in a "value" property, and add a forwarding for compat

css-meeting-bot avatar Jul 31 '20 17:07 css-meeting-bot

Could someone describe the "forwarding"? Is the plan to continue to use indexed getters/setters? That would be sad, for a relatively-new API with only a single implementation to do so.

domenic avatar Jul 31 '20 17:07 domenic

Yes, that's what we mean. I'm gonna ask for some use-counters to see if they're needed.

tabatkins avatar Jul 31 '20 22:07 tabatkins