#223: TAG review for CSS Typed OM

Visit on Github.

Opened Jan 8, 2018

Hello TAG!

I'm requesting a TAG review of:

We'd prefer the TAG provide feedback as (please select one):

  • open issues in our Github repo for each point of feedback
  • open a single issue in our Github repo for the entire review
  • leave review feedback as a comment in this issue and @-notify [github usernames]

Discussions

Comment by @dbaron Jan 26, 2018 (See Github)

I've started reading through the spec and filing some issues; I haven't gotten that far yet, though, but I plan to continue.

Comment by @dbaron Jan 28, 2018 (See Github)

I'd also note that some of the open issues already pointed out in the spec are reasonably serious, such as pointing out that parts of the spec aren't written yet. That's a bit of an obstacle to getting interoperable implementations. Hopefully these will get sorted out, though.

Comment by @darrnshn Jan 28, 2018 (See Github)

Thanks for starting a review! Yes, we are hoping to resolve these issues by the end of this week.

Comment by @tabatkins Jan 28, 2018 (See Github)

(In particular, I'm in Sydney for the week specifically to burn thru the issues list.)

Comment by @foolip Jan 30, 2018 (See Github)

FYI, there's a Intent to Ship: CSS Typed OM on blink-dev which links here.

@dbaron, do you expect to file more issues? (I see issues from 2 and 4 days ago, not clear if the latest was the last.)

Comment by @dbaron Jan 30, 2018 (See Github)

I'm hoping to file more (or at least, hoping to read more of the spec), perhaps this afternoon. It's a pretty big spec!

Comment by @foolip Jan 30, 2018 (See Github)

Sound good, thanks!

Comment by @torgo Jan 31, 2018 (See Github)

Taken up at London F2F.

Comment by @slightlyoff Jan 31, 2018 (See Github)

Per conversation at F2F, the motivation and design tradeoffs need to be documented. An Explainer or use-cases doc would resolve.

Comment by @annevk Jan 31, 2018 (See Github)

Note that https://github.com/heycam/webidl/issues/345 is worth reviewing here too. In particular the introduction of new objects that require a JavaScript Proxy to implement. As far as I know the last time we discussed this with TC39 they were still against that (and definitely far from consensus).

Comment by @nainar Feb 2, 2018 (See Github)

@slightlyoff - I have a PR for an explainer out here, in case you want to take a look.

Sorry it wasn't part of the original design review.

@foolip I will update the intent to ship thread once this is committed.

Comment by @slightlyoff Feb 2, 2018 (See Github)

Thank you for the explainer!

Comment by @dbaron Feb 2, 2018 (See Github)

@annevk, is https://github.com/w3ctag/design-reviews/issues/223#issuecomment-361896008 addressed at all by this change? If not, which objects are the relevant ones?

Comment by @slightlyoff Feb 2, 2018 (See Github)

We're starting to review at the London F2F; some stream-of-consciousness questions:

  • Is CSSStyleValue really meant to be available in all Worker types? E.g., does it make sense in Service Workers?
  • Glad to see constructors on the concrete types!
  • Loving the static methods for shorthands (e.g. CSS.px()).
  • Don't understand how Example 7 works: it looks like the example should work with styleMap, but instead it uses attributeStyleMap, which doesn't seem like it should have values for 'object-position'
  • Should computedStyleMap() be synchronous? It seems to flush style on read...is that right? And does it need to be a function?
  • ...that gets us to a perhaps bigger question: should the OM be trying to help ensure performant read-back and separation of writes/reads? If this is a chance to go that way, it won't come around again. Has it been discussed?
  • to @annevk's point, it looks like CSSNumericArray is a duck-type now. :shrug:
  • @plinss points out that [[tokens]] is a list of strings and not a list of actual tokens. Another way to look at this is that the tokenizer isn't exposed. Should it be?
  • Are changes to CSSVariableReferenceValues observable without polling?
  • Are there concrete performance numbers we can point to that show the benefits of the Typed OM approach?

Thanks!

Comment by @annevk Feb 2, 2018 (See Github)

@dbaron all objects that use getter/setter: CSSUnparsedValue, CSSNumericArray, and CSSTransformValue. See also https://github.com/heycam/webidl/issues/100 about more clearly marking getter/setter as legacy in IDL.

Comment by @slightlyoff Feb 2, 2018 (See Github)

@annevk: this doesn't appear to require an proxy in implementations. Here's the Chrome code (auto-gen'd binding output), e.g.

Comment by @annevk Feb 2, 2018 (See Github)

@slightlyoff implementations have shortcuts, sure. This is assuming we'd use actual JavaScript to represent these objects, as per the extensible web principles.

Comment by @annevk Feb 2, 2018 (See Github)

Also, my objection is not that they require proxies were they to be implemented in JavaScript, my objection is that we've been explicitly asked to avoid introducing any such objects going forward by TC39.

Comment by @nainar Feb 5, 2018 (See Github)

@slightlyoff in response to your questions:

We're starting to review at the London F2F; some stream-of-consciousness questions: Is CSSStyleValue really meant to be available in all Worker types? E.g., does it make sense in Service Workers?

Nope, it should be exposed to the Paint API only. Filing an issue on typed om: https://github.com/w3c/css-houdini-drafts/issues/632

Don't understand how Example 7 works: it looks like the example should work with styleMap, but instead it uses attributeStyleMap, which doesn't seem like it should have values for 'object-position'

The example seems incorrect - it should be checking computedStyleMap(). Filed an issue here: https://github.com/w3c/css-houdini-drafts/issues/633

Should computedStyleMap() be synchronous? It seems to flush style on read...is that right?

IIUC: when you read values of computedStyleMap() it causes a style recalc, so yes it is synchronous.

And does it need to be a function?

The CSSWG resolved to have it be a function, IIRC. For reasons:

  • To mirror getComputedStyle()
  • To reflect that it will return a new StylePropertyMap each time. @tabatkins

...that gets us to a perhaps bigger question: should the OM be trying to help ensure performant read-back and separation of writes/reads? If this is a chance to go that way, it won't come around again. Has it been discussed?

I don't understand the question here. Could you elaborate?

@plinss points out that [[tokens]] is a list of strings and not a list of actual tokens. Another way to look at this is that the tokenizer isn't exposed. Should it be?

Exposing a tokenizer was delayed to a Level 2 spec. Context here: https://github.com/w3c/css-houdini-drafts/issues/193

Are changes to CSSVariableReferenceValues observable without polling?

IIUC. No, the CSSVariableReferenceValues objects aren't live, you have to get the new value again, if changed.

Are there concrete performance numbers we can point to that show the benefits of the Typed OM approach?

Since there is a lot of discussion about this, I am filing an issue to track the discussion: https://github.com/w3c/css-houdini-drafts/issues/634

Thanks!

Comment by @RByers Feb 14, 2018 (See Github)

Also, my objection is not that they require proxies were they to be implemented in JavaScript, my objection is that we've been explicitly asked to avoid introducing any such objects going forward by TC39.

@annevk can you link to any reference documenting the level of consensus on that? Is there a recommended alternative way already defined to achieve a similar design to existing web platform APIs (like TouchList)? Without there being a viable alternative ready today, I don't think I can reasonably deny requests to ship additional such APIs in blink.

Discussed Mar 6, 2018 (See Github)

David: I have reviewed some of this (about half the spec). It's a pretty big spec--attempt to replace CSSOM with something we like better. Some other Mozilla folks have also reviewed due to vendor opinion requests... A couple of issues seemed to rise to the top:

  1. Reason was for better performance/ergo for CSS manipulation than working with strings. Wins on ergonomics, but not winning on perf by the evidence.
  2. Spec is not yet solid-enough to lead to two interoperable implementations.
  3. Few other things maybe not worth discussing.

Alex: we left a lot of feedback from our last F2F... Have recieved some responses already. HAve you processed these?

David: Some were design issues... 23-30 issues + Anne/Boris a similar amount. Spec has had huge changes since the F2F--probably Tab's full-time effort.

Issue: Lots of undefined--what's the underlying model, how is it defined. Spec defines the object model API, but not the model that is being defined. Doesn't say what happens to the model when you change things.

Alex: To a certain extend, browsers don't open up their CSS...

David: More like... complex properties (like transform) have parts, what's the model for how the underlying model for the transform is manipulated, and how the string version is affected, etc.

Alex: We've seen that ambiguity in other parts of the DOM (Attributes/properties). Interop testing has ironed this out... Perhaps implementations have to figure out how that works themselves?

David: Some needs to be fixed in Typed OM, others may need to be distributed to other specs where applicable. Probably other parts need to be defined in CSSOM where serialization is defiend. There's an existing mess, we dont' want to add to the mess without making it better.

Alex: I'm concerned about holding this up for some undetermined long time. I'm also concerned about the lack of definition of a model.

Travis: We should be sure there is clear application of a model.

David: If Chrome implements out ahead, we could get stuck with Chrome's interpretation of

Hadley: Perhaps "OM" is not reflecting the nature of the spec very well... add "API" suffix?

Alex: Worried that Anne's concern is theoretical...

Peter: looks like there are actual side-effects.

Alex: Think we should get to resolutions by writing tests.

Peter: any given property needs to have definition of how they are represented in the object model... we do not want to hold up the rest of this spec until that happens.

Alex: Do we need to channel this feedback to the CSS WG?

Peter: there was a question about needing Proxies to effect this (as it may be slow).

Alex: Chrome doesn't use a proxy.

Peter: Houdini meeting coming up. We can relay the feedback about seeing a consistent OM defined for what parts get set when properties (parts of properties) are changed. David?

David: Nothing else I want to raise here; brought some other issues up to the CSS WG as a whole. Should I write-up the feedback?

Peter: I can do it.

(Note that https://github.com/w3c/css-houdini-drafts/issues/718 may also cover some of this.)

Comment by @torgo Jul 25, 2018 (See Github)

https://github.com/w3c/css-houdini-drafts/issues/718 https://github.com/w3c/css-houdini-drafts/issues/574

Comment by @dbaron Aug 14, 2018 (See Github)

I'm still concerned about seeing progress in w3c/css-houdini-drafts#718, but we're going to close this issue at this point because we don't feel we need to cycle back to it for further TAG discussion. Thanks for requesting TAG review.