Re: [css-houdini-drafts] When doing CSSStyleValue.parse(), what should throw vs return null?

The CSS Working Group just discussed https://github.com/w3c/css-houdini-drafts/issues/305, and agreed to the following resolutions:

```
RESOLVED: Be consistent in all thre cases; follow TAG guidelines once they have them
```

<details><summary>The full IRC log of that discussion</summary>

```
<TabAtkins> Topic: https://github.com/w3c/css-houdini-drafts/issues/305
<fantasai> TabAtkins: When you call CSSStyleValue.parse()
<fantasai> TabAtkins: parses arbitary stirng into type doOM value
<fantasai> TabAtkins: Takes a property name and a value
<fantasai> TabAtkins: Several wasy to do something wrong, not sur ewhethe rto thorw or eturn null
<fantasai> TabAtkins: 1st case is, property is not even an identifier
<fantasai> SimonSapin: If property is a separate arg, not parsed as "prop:val"
<fantasai> SimonSapin: Then do you even need to parse it?
* astearns minute-man added basically an empty comment for the first instance
<fantasai> SimonSapin: For CSS.supports(), we have API with 2 variations
<fantasai> SimonSapin: One with colon syntax, uses CSS sntax
<fantasai> SimonSapin: Other one passes prop and value separately
<fantasai> SimonSapin: In that case you don't parse it
<fantasai> TabAtkins: ...
<fantasai> SimonSapin: Just assume it's an ident
* dbaron deleted the empty comment... but it turns out that that's the danger of using Topic: rather than Github topic:. Perhaps that was a mis-feature?
<fantasai> TabAtkins: Yes, that avoids the issue entirely
<fantasai> iank_: Might want to throw on the empty string
<fantasai> iank_: We throw a SyntaxError on that
<fantasai> SimonSapin: empty string is not valid property name
<fantasai> iank_: It's not a valid function name
<fantasai> SimonSapin: Are we talkig about property names
<fantasai> iank_: First argument in custom paint is an ident, same issue
<fantasai> shane: What does supports() do with empty string?
<fantasai> SimonSapin: supports method, you can give it custom property, in which case name starts with --
<fantasai> shane: There are 3 differnet erro conditions
<fantasai> shane: 1st, not a valid proeprty name. If tha'ts just empty string, sitll
<fantasai> shane: 2nd, valid property name, but doens't exist. Can only happen for non-custom properties
<fantasai> SimonSapin: could treat those the same
<fantasai> shane: 3rd, valid property name, but value's grammar doesn't match
<fantasai> shane: I thought 2 & 3 were the same, but both f them could be that you're simply running on a browser that hasn't built support for that syntax yet
<fantasai> TabAtkins: Taking the property name makes it not an error
<fantasai> shane: Could treat them all the same, and thorw
<fantasai> s/thorw/throw/
<fantasai> Rossen: It would be easier to handle now from user side
<astearns> s/now/null/
<fantasai> Rossen: Rather than wrapping everything in try for parsing all over the place
<fantasai> Rossen: 1 & 2 are more of an error than 3
<fantasai> Rossen: 3 is you'r eparsing a value that you can't prase
<fantasai> shane: But both 2 & 3 could be that you're trying to parse a feature that the browser doesn't yet support
<fantasai> dbaron: It does feel like the more CSS-ish thing ot do, not to throw for unsupported values or properties
<fantasai> dbaron: In normal CSS, they get silently dropped
<fantasai> dbaron: so null seems more sensible
<fantasai> shane: Note that we throw if you set a value that doesn't parse according to your type already
<fantasai> dbaron: If mistyped things throw, then unknown things should throw. I would agree wiht that
<fantasai> philipwalton: My intuition is, you don't want to both try-catch and check for null when you're writing code
<fantasai> philipwalton: I think htere are many cases you want try-catch, so thorwing an error is probably best
<fantasai> shane: Rosen, you were leaning towards null before. What do you think now?
<fantasai> Rossen: Not gonna object, but not gonna love it.
<fantasai> iank_: What's your hesitation
<fantasai> Rossen: More of a philosophical discussion
<fantasai> Rossen: I'm very biased to the APIs we work on on the windows platform
<fantasai> Rossen: We have strict principles on what is an error, how to deal with errors, how to make API that is ergonomic for developers
<fantasai> Rossen: Don't watn to sprinkle try-catch all over, half is error-handling, half is business logic
<fantasai> Rossen: We have strict rules to guide our APIs. Not gonna impose those here.
* astearns what about not throwing, but returning a value that throws an error when you try to read it? Could that be the worst idea?
<fantasai> fantasai: Might be worth having that philosophical discussion
* TabAtkins Worst idea, yes.
<fantasai> fantasai: and codify some principles for our APIs, whether same or different from Windows API principles
* TabAtkins Value that can validly be used, but represents a random property/value.
<fantasai> shane: Inconsistent to return null here.
<fantasai> shane: You then set this value, want to throw an error here
* fantasai missed exaclty the nuance htere
<fantasai> iank_: Does TAG have any guidance?
<dbaron> I think Shane was saying that it's not as inconsistent as he thought it was before.
<fantasai> plinss: We have a document discussing principles for designing APIs, but don't have anything on this topic
<shane> yup. When we set values we need to throw an exception on error because there's no return value
<dbaron> because the other case was setting an object and getting a type mismatch, whereas this is parsing a string.
<fantasai> Rossen [offers to review that? send feedback? something?]
<shane> but here we're getting the result as the return value so we can use null to signal an error
<fantasai> Rossen: So going back
<fantasai> Rossen: Are you reverting your opinion about throwing vs null?
<fantasai> shane: Yeah
<fantasai> shane: I'm going back from saying we should throw to not having a strong opinion
<fantasai> Rossen: Does anyone have a strong opinion?
<fantasai> philipwalton: Likely to do this in environment where you don't know what your'e getting
<fantasai> philipwalton: so maybe throw
<fantasai> Rossen: Think about it from a mid-layer library, start exposing some of those
<fantasai> Rossen: First they're going to write their own wrapper that does a throw-catch
<fantasai> Rossen: are going to call that so they don't have to deal with throw/catch all over
<fantasai> Rossen: If that's the principle we design around, okay
<fantasai> Rossen: ...
<fantasai> fantasai: Sounds like we should table discussion and have your philosophical discussion about throw vs null?
<fantasai> plinss: To me, throwing is semantically different than returning an error
<fantasai> plinss: If you pass in a property that isn't an ident, e.g. numbers, cna't possibly be a rpoperty name. Could see an error
<fantasai> pl But if passing a name that's unknown to the endgine, then return null
<fantasai> PpliNot recommending htat's what we do, but there's a distinction between throwing errors and returning null, and it's useful to make that disuction
<fantasai> TabAtkins: If you do construct a value that doesn't make any sense, and you return null. Whatever you do with that will throw an error
<fantasai> TabAtkins: All you can do is check for null
<fantasai> plinss: Throwing is ofr exceptional  circumstances like out of memory
<fantasai> plinss: I've tried using it for everthing, and that way lies madness
<fantasai> plinss: For a certain class of things, throw exceptions, and be consistent about it.
<fantasai> plinss: We need some guidance on that, and we don't have it
<fantasai> iank_: I was just looking through a few of the Web apis, like json.parse
<fantasai> iank_: Dae.parse() doe snot ... seems like more things throw than silently fail
<fantasai> iank_: HTml throws if not well-formed
<fantasai> iank_: Could file an issue on the TAG
<fantasai> shane: Let's do that
<fantasai> Rossen: Let's resolve with following -- in all three cases we will be consistent in how we handle the condition
<fantasai> Rossen: And we will follow the TAG guideline on whether to throw or return null
<dbaron> The TAG guidance might not be all that prescriptive...
<plinss> https://github.com/w3ctag/design-principles/issues/55
<fantasai> RESOLVED: Be consistent in all thre cases; follow TAG guidelines once they have them
```
</details>


-- 
GitHub Notification of comment by css-meeting-bot
Please view or discuss this issue at https://github.com/w3c/css-houdini-drafts/issues/305#issuecomment-294668245 using your GitHub account

Received on Tuesday, 18 April 2017 03:49:32 UTC