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

CSS Properties and Values API Level 1 #318

Closed
1 of 3 tasks
andruud opened this issue Nov 1, 2018 · 14 comments
Closed
1 of 3 tasks

CSS Properties and Values API Level 1 #318

andruud opened this issue Nov 1, 2018 · 14 comments

Comments

@andruud
Copy link

andruud commented Nov 1, 2018

こんにちは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 @andruud
@slightlyoff
Copy link
Member

Hey @andruud,

This just came back through the Blink API OWNERS review process and there's a concern that this wasn't filed at the Intent-to-Implement stage. Can you perhaps update this issue to note when you'd like to receive a review by? It's my understanding that we (the TAG) are in your critical path to ship, but nothing here says as much.

Thanks

@andruud
Copy link
Author

andruud commented Dec 27, 2018

@slightlyoff My apologies for late reply. I've been OOO.

  • Issue updated with some minimal sense of urgency.
  • The i2i (and early implementation work) was done by someone else, I can't answer questions re. lack of filing at that stage.

@travisleithead travisleithead self-assigned this Jan 22, 2019
@dbaron dbaron self-assigned this Jan 22, 2019
@plinss plinss added this to the 2019-02-05-f2f milestone Jan 22, 2019
@torgo torgo assigned alice and hober and unassigned travisleithead Feb 5, 2019
@alice
Copy link

alice commented Feb 5, 2019

Hey @andruud,

Thanks for getting back to us. David and Travis are assigned to the review, but be advised that Travis and Alex are cycling out of the TAG as of this week.

As you may have seen from other review threads in this repo, it's important for us to understand the motivating use-cases for a feature we're looking to review. The spec itself has relatively minimal example code and the introduction section outlines the layering it provides (which seems great!), but doesn't indicate why this is powerful or helps authors.

Our general guidance is that teams should try to frame this part of their design in an Explainer:

https://github.com/w3ctag/w3ctag.github.io/blob/master/explainers.md

These documents focus on the problem being solved rather than the details of how a solution fits into existing specs.

We can proceed without an explainer, but if you are able to take a look at the template and pull something together (based on notes from your previous design discussions, for example), that will most likely speed up the process by answering likely questions ahead of time, and giving us more context for the review.

In the meantime, we managed to find something that functions like a partial explainer for your feature:

https://danielcwilson.com/blog/2018/02/houdini-quickstart/

Several questions arise from this outline:

  • What's the behavior of a custom property that is used in CSS before it is defined in script? If a value or property is grabbed via the OM before it's defined, what is its representation? After? HTML Custom Elements have an "upgrade" phase; is there an equivalent? The discussion in 2.2, Example 2 hints at the problem but the example code doesn't handle it
  • How are the syntax classes defined? Is it possible to define new classes from script?
  • Why are the syntax classes defined as strings?
  • Is there a CSS Type'd OM version of this API? Could the syntax values be taken from TypedOM types?

Thanks for asking for feedback and for working with us.

@andruud
Copy link
Author

andruud commented Feb 11, 2019

Thanks @alice,

Q: What's the behavior of a custom property that is used in CSS before it is defined in script? (etc)

I assume you mean the case where a custom property is first declared somewhere, and then some time later a syntax is registered for that custom property, while the pre-registration value still exists.

It's important to note that registering a property does not affect the cascade. This means that a custom property that has its specified value "invalidated" by an after-the-fact registration still has that same "would-be-invalid" specified value after the registration. Only when calculating the computed value is the registered syntax (if any) taken into consideration. This behavior ensures that we don't have to reparse/recascade when registering properties (only recalculate computed values). I thought the spec handled this problem well enough, but apparently we can improve there.

Now, for the parts not handled at all by the spec:

CSS OM:

  • Registering a property does not change CSS OM behavior when getting values from declared CSSStyleDeclaration objects. This means that if you first declare --x: 10px in a stylesheet, then register --x with the syntax "<color>" (for example), and then grab the value via CSSStyleDeclaration, it will still produce 10px (and not an error or an empty string, for example).
  • Registering a property does change CSSOM behavior when setting values on declared CSSStyleDeclaration objects. If --x is registered with "<color>", an attempt to set a value that does not match the "<color>" syntax will fail. This behavior is not very consistent with "syntax ignored until computed-value-time", and I'm not sure if I like it, but it's what Tab wanted.
  • Registering a property changes the behavior of computed CSSStyleDeclaration objects, but that should follow from 2.4 Calculation of Computed Values.

CSS Typed OM:

  • When grabbing an already-defined value via declared StylePropertyMaps after registering, the value is parsed (on its way out of the API) against the syntax, and then reified according to type of the value that was parsed. If that parsing fails, the value is reified as if the custom property is not registered.
  • When setting values post-registration, the incoming CSSStyleValue must match the registered syntax.
  • When grabbing values from the computed StylePropertyMapReadOnly, the value is reified according to the type of the underlying value (which again is determined by the registered syntax).

Q: How are the syntax classes defined?

In section 2.3. "Supported syntax strings"? :-)

Maybe I don't understand the question.

Q: Is it possible to define new classes from script?

No.

Q: Why are the syntax classes defined as strings?

The syntax string itself has internal syntax. E.g. "<length>+ | <percentage># | auto" describes a space-separated list of lengths, or a comma-separated list of percentages, or auto. This can't easily be represented by e.g. an enum.

Also:

  • We want to be flexible to allow for more complicated syntax in the future. I think there is a secret desire to eventually support the full CSS metasyntax.
  • Whatever solution we have for specifying the syntax must also work for the planned declarative version of the API, and should be as similar as possible to the JS API.
  • There is an open issue to describe the grammar of the syntax string. This issue is currently blocking shipping in Chrome.

Q: Is there a CSS Type'd OM version of this API?

No. I don't know if it's appropriate for CSS Typed OM to be involved here or not (specifically w.r.t. the syntax definition). Apparently there has been discussion around this before (using JS objects to define syntax), but the conclusion seems to be that we don't want that.

@slightlyoff
Copy link
Member

Hey @andruud:

Thanks for the detailed response. In general, it sounds like you've considered most of the points raised. The temporal behavior you outline for CSS OM and CSS Typed OM are internally consistent and make sense. Thanks for the explanation!

Regarding extensibility, I'm personally happy to see the current system move forward if and only if you and the WG are committed to a script-driven extensibility framework for new syntaxes. It is strange that these formats are welded shut in a fixed vocabulary that can't be desugared to script, so planning on opening up the system in future seems like the best path forward.

If that isn't going to be possible (e.g., invoking script at style resolution time is somehow never going to work), would love to know that now.

Thanks again.

@andruud
Copy link
Author

andruud commented Apr 26, 2019

@slightlyoff, sorry for the delay, I had to think about this a little.

if and only if you and the WG are committed to a script-driven extensibility framework for new syntaxes

OK, let's explore this then. It would be some worklet operating at the css-syntax level, I suppose, with some "token API" allowing you to either produce some (TypedOM?) value from a sequence of tokens, or not. Also, the worklet must define interpolation behavior between values.

I'm still not sure if this is "going one level to deep", or not. (What's next? Tokenization API? Input Stream API? An API to extend the Standard Model? We have to stop somewhere).

In any case, I'll draft a proposal and see what happens. :-)

If that isn't going to be possible (e.g., invoking script at style resolution time is somehow never going to work), would love to know that now.

It's possible.

@torgo
Copy link
Member

torgo commented Jul 3, 2019

@andruud we are just circling back to this on today's TAG call. You noted in your comment above that you were going to draft something. Did that happen? Could you drop a link to that here?

@dbaron
Copy link
Member

dbaron commented Jul 19, 2019

We're trying to figure out to what extent this review should include the unmerged PR at w3c/css-houdini-drafts#847 and the various issues related to it such as w3c/css-houdini-drafts#902 (which proposes revisiting w3c/css-houdini-drafts#880) and w3c/css-houdini-drafts#845 and w3c/css-houdini-drafts#846.

@alice
Copy link

alice commented Jul 19, 2019

It would also be good to get an update on the implementation/shipping status.

I noticed that the intent to ship thread for Blink had 3 LGTMs, but then @andruud noted that the concern raised by @emilio required further thought, leading to the issues and PR @dbaron linked to above.

@andruud
Copy link
Author

andruud commented Jul 19, 2019

@andruud we are just circling back to this on today's TAG call. You noted in your comment above that you were going to draft something. Did that happen? Could you drop a link to that here?

@torgo I have not yet done this. But that would anyway be a separate spec.

We're trying to figure out to what extent this review should include the unmerged PR at w3c/css-houdini-drafts#847

Hmmm, what I proposed in that PR may or may not be similar to what we end up merging. :-) If you prefer I can request a separate TAG review for @property when the time comes.

w3c/css-houdini-drafts#902

update on the implementation/shipping status

Yes, the feature was enabled and about to ship in Chrome, but I disabled it again because of w3c/css-houdini-drafts#902.

The issue is that the spec currently requires some non-computed style APIs to be aware of property registrations. Specifically, 1) reification behavior for specified styles, 2) setProperty behavior, and 3) The two-param form of CSS.supports (described in css-conditional, I believe, but that spec seems to down right now).

This is not a problem for CSS.registerProperty, as the registration is effectuated immediately when the JS call takes place. However, for @property (w3c/css-houdini-drafts#847), the rules cascade and whatnot, and do not take effect until "style data is updated" (as @emilio calls it). This means that calls like setProperty would need to synchronously update that style data to know the current set of registered properties, and having to do that synchronously is new and undesirable behavior. The setProperty case is also especially bad, since it immediately invalidates the same data you synchronously "flushed".

Hence, we want to specify that APIs don't depend on the registrations (w3c/css-houdini-drafts/pull/912), except for APIs which retrieve the computed style (as these already compute the style synchronously). This would make everything very consistent technically, and accurately represents the actual parse-time behavior of registered custom properties, but it makes Typed OM less useful for specified style, since registered custom properties would be treated as unregistered. The registered syntax of a property would be an aspect of computed-value time only, and hence that information is unavailable in other circumstances.

But @tabatkins doesn't like this, therefore this is going nowhere at the moment. Ideally we would have both nice Typed OM/API behavior and a technically consistent model, but I don't see a way to do that.

Anyway, that's the shipping status. 😿

@andruud
Copy link
Author

andruud commented Jul 25, 2019

Update: @tabatkins fixed all the things ( <3 ). Shipping is now imminent.

@dbaron The review should now probably include @property, since that PR was merged (sooner than I expected). The other issues you mentioned are also resolved, except w3c/css-houdini-drafts#846, but I expect that issue will close soon too.

@andruud andruud mentioned this issue Aug 7, 2019
5 tasks
@andruud
Copy link
Author

andruud commented Aug 7, 2019

I filed a separate review request for @property: #402. If TAG feels this is excessive and/or unnecessary, just close it.

@torgo
Copy link
Member

torgo commented Aug 28, 2019

On our teleconf today, discussing if we can close this particular issue?

@hober
Copy link
Contributor

hober commented Sep 10, 2019

Hi,

A few of us have been looking at this again in a breakout. Overall we’re really happy with the progress made, and the continuing discussion and work happening on this in various issues and PRs, and we hope that discussion continues.

Thanks again for filing the @property review separately.

We have no specific additional feedback, except to say that an explainer would have been extremely helpful.

@hober hober closed this as completed Sep 10, 2019
@dbaron dbaron added the Resolution: satisfied The TAG is satisfied with this design label Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants