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-typed-om] Change CSSKeywordValue's attribute to allow forward-compatible upgrades #735

Closed
tabatkins opened this issue Mar 22, 2018 · 7 comments

Comments

@tabatkins
Copy link
Member

tabatkins commented Mar 22, 2018

Many properties today accept single keywords. These are well-handled by CSSKeywordValue objects.

Some, tho, are more complex. 'font-synthesis', for example, has the grammar weight || style || small-caps, so you can provide 1-3 keywords. How best to reflect this? Two ways I can see right now:

  1. When the value is a single keyword, reflect as CSSKeywordValue; when two or more, reflect as a specialized object type (currently just the generic CSSStyleValue).
  2. Always reflect as a specialized object type (or, currently, generic CSSStyleValue).

(2) gives authors a consistent way to interact with the value - whichever way the API provides for you to ask "is small-caps in the value?", you can use that no matter what the value is. (1) is less consistent, but allows for more convenient authoring in simple cases.

The problem here is that a property might start as a single keyword, and then later be upgraded to a more complex value. This has happened many times in CSS's history. So backwards-compat might force us into (1) for a particular property, even if we'd have chosen (2) if we were designing the property's reification from scratch. That suggests that, for overall consistency, we should choose (1) for all properties now, so we future changes give a consistent result rather than messy legacy.


So, my suggestion: we should make the common general-purpose types (CSSKeywordValue and CSSNumericValue) have distinct and memorable attributes which aren't likely to conflict with more specialized object types in the future.

That way, when we do upgrade a property to a more complex grammar, we can define that the new specialized object also has a convenience attribute matching the appropriate general-purpose type, reflecting the same as the general-purpose type would as long as the value is simple enough to match.

To be more concrete, we can leave CSSUnitValue how it is, and change CSSKeywordValue to have a .keyword attribute instead. (CSSStringValue, when we add it, would have a .string attribute.) Then the special CSSFontSynthesisValue type would (a) expose a set for the keywords it contains, and (b) also have a nullable .keyword attribute which, when the value it's reifying is just a single keyword, would contain that keyword.

Code that was written assuming that the property only took a single keyword (and thus reified as a CSSKeywordValue) would continue to work, as long as the page continues to only use a single keyword. (If the page starts using the new values, any code assuming the old values would break, so that's not a concern.) This should make upgrading properties much less fraught with back-compat concerns, which is a big win.

@darrnshn
Copy link
Collaborator

darrnshn commented Mar 22, 2018

So to check my understanding: suppose a property only took single keywords and we reified them as CSSKeywordValues. The property is then changed to allow multiple keywords. Then there are two possible approaches that I can think of:

  1. We still reify single keywords as CSSKeywordValues, but reify multiple keywords as a new type ComplexValue. Upside is 100% backwards compat (AFAICT). Downside is that writing generic code that works on both CSSKeywordValue and ComplexValue is difficult as you have to handle both types. Another downside is that new types can accumulate over time.

  2. [your suggestion I think?] Alternatively, reify both single & multiple keywords as a new type ComplexValue. Backwards compat as long as no one is relying on the types staying the same (which I believe they're not supposed to anyway). Any operation that was valid for CSSKeywordValue must also be valid for ComplexValue. In other words, ComplexValue "implements the CSSKeywordValue interface". Downside is that we have to carefully consider backwards-compat when naming (e.g. .value vs .keyword).

For me, I agree option 2 sounds better, but there might be more options that I haven't thought of.

@tabatkins
Copy link
Member Author

Yes, that's it exactly. Old code can pretend that ComplexValue is a CSSKeywordValue (or, depending on what the property's grammar was before it got extended, a CSSUnitValue or CSSStringValue), and it'll act exactly correct. New code can interact with ComplexValue in the new ways that the new class offers, giving them more powers.

I wonder if the right way to go about it is to actually make them subclasses of CSSKeywordValue/CSSMathValue/etc.

@tabatkins
Copy link
Member Author

Hmm, no, CSSMathValue has 7 concrete subclasses, descending it from any particular one is kinda weird. I'll have to think on this a bit.

@darrnshn
Copy link
Collaborator

Hmm also, does JavaScript support multiple inheritance? What if we had to create a new type that is both a CSSKeywordValue and a CSSUnitValue (not sure if this could happen tho).

@tabatkins
Copy link
Member Author

It doesn't support multiple inheritance, no. We'd just have to suck it up in that case. :(

(It could happen with any property that currently has a syntax like auto | <length>, and later gets something more complicated.)

@css-meeting-bot
Copy link
Member

The Working Group just discussed Forwards-compat design choices, and agreed to the following resolutions:

  • RESOLVED: when we have this sort of upgrade situation we should, if possible, make new complex type subclass whatever it was before, but only do it for upgrade situations
The full IRC log of that discussion <dael> Topic: Forwards-compat design choices
<TabAtkins> GitHub: https://github.com//issues/735
<dael> TabAtkins: One issue...a general issue with any attempt to design typed om is what happens when we change value space of a property.
<dael> TabAtkins: There are patterns we commonly do like when something is a list. There are things not handled well like changing a property from being a single keyword to something more complex. Which means...the question is hwo to handle reification. If it started simple and we reify as a keyword and it becomes more complex there's options
<dael> TabAtkins: Firs tis we maintaing compat. If it's simple it's a keyword and more complex it's more complex. When the single keyword isn't a special case it's awkward to handle. It would mean if you want to test if the weight value is spec is it' the keyword or the keyword set. You'd have to branch twice. If we know the final value it would be more complex.
<dael> TabAtkins: I think I can make it less complex in common cases. In the basic types the relevent accessor have a unique name and then when it upgrades is still has the accessor.
<dael> TabAtkins: If we start with font synth as a single value and then becomes a double it would be a keyword set with a .keyword property that reflects the single value that exists. That way any older code will continue working properly. Newer code can work more natively.
<dael> TabAtkins: Won't work always, but a lot of cases. Propobaly changes the accessor to .keyword and string to .string. .unit.value stays where it is.
<dael> TabAtkins: More basic cases like a hash token we'll add a unique value.
<dael> TabAtkins: Is that a good idea or do we accept that as we extend we'll have legacy things.
<dael> shane: Is there a combo where we could consider so the special object type extends. THis is the same trick sa the style mapping. That gives us the consistancy approach to a longer list
<dael> TabAtkins: Likely. What I like is if a property is keyword or numeric and people branch on type can still branch on type.
<dael> TabAtkins: Means a slightly weird object hierarchy and we can't do it on number...Well...math values ar emore complex. But strings or keywords subclassing keyword value class would be fine.
<dael> dbaron: I feel...there's compat risk. Also the risk that once a thing is upgraded that people will write new code that only works for old stuff.
<dael> TabAtkins: ExmpalE?
<dael> dbaron: You're writing new code tha tonly works with single keyword case because you forgot this property was upgrade. When the compat thing is a thing where you can handle the old values you're creating a risk that people will fall into the compat risk.
<dael> TabAtkins: True, but only way to handle is break old code.
<dael> dbaron: True.
<dael> dbaron: There might be an advantage to not having the compat for properties that weren't upgraded. So if a property starts as multi keywords there's an advantage ot not having single keyword available.
<dael> TabAtkins: That was my plan. If we know ahead of time something is complex it'll descend straight from style.
<dael> TabAtkins: That reflects on a 3rd option, similar to how Ana tried to handle this. Every value had a short named accessor that gave you a complex value of it. If it was simple you'd get simple, but you could also ask for hte complex form and if it existed you'd get it.
<dael> fremy: Other similar thing was a problem for c# browser. They solved that by when you add a value you give a list of all functions you expect and then as the UA you know all the values expected byt he author and if it's none you can return the css tyle value. You have context. It worked for c# brwoser
<dael> TabAtkins: You'd have to put that into the get method
<dael> fremy: i don't know if it's good design, but it's one way that works.
<dael> TabAtkins: I thought of that, but figured it was too much to write on every get call.
<dael> TabAtkins: So, taling this over. Best idea is when we have this sort of upgrade situation we should, if possible, make new complex type subclass whatever it was before, but only do it for upgrade situations.
<dael> fremy: Another thing we could do is when we make a breaking change we can make a priority like a .v2 if they don't have it they get the old scenario.
<dael> TabAtkins: That's true. THat's reasonable. I hadn't thought of the escape when we can't upgrade.
<dael> TabAtkins: Reasonable to people?
<dael> Proposed resolution: when we have this sort of upgrade situation we should, if possible, make new complex type subclass whatever it was before, but only do it for upgrade situations
<dael> RESOLVED: when we have this sort of upgrade situation we should, if possible, make new complex type subclass whatever it was before, but only do it for upgrade situations
<dael> emilio: How is clampping done in typed om?
<dael> TabAtkins: It's automatic. When it enters the style system clamping happens. If you try and set a negative value typed OM is fine, when it enters style system it's 0.

@tabatkins
Copy link
Member Author

Closing as it looks like our discussion ended on no current change.

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

3 participants