search

w3ctag / design-reviews

W3C specs and API reviews
Creative Commons Zero v1.0 Universal
331 stars 56 forks source link

CSS Properties and Values API Level 1 #318

Closed andruud closed 5 years ago

andruud commented 5 years ago

こんにちはTAG！

I'm requesting a TAG review of:

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

slightlyoff commented 5 years ago

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 commented 5 years ago

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

alice commented 5 years ago

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:

Thanks for asking for feedback and for working with us.

andruud commented 5 years ago

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:

CSS Typed OM:

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:

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 (https://github.com/w3c/css-houdini-drafts/pull/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 commented 5 years ago

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 https://github.com/w3c/css-houdini-drafts/issues/846, but I expect that issue will close soon too.

andruud commented 5 years ago

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

torgo commented 5 years ago

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

hober commented 5 years ago

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.