Drop keyframesName function, exposing KeyframesName constructor instead. #34
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This change drops the
keyframesName
function and exposes theKeyframesName
constructor in its place.Design considerations
First, a little background: The
keyframesName
function was intended as a smart constructor "placeholder" of sorts. I got the idea that it might be important when I noticed that a<keyframes-name>
value is a<custom-ident>
meaning that certain values aren't allowed, such as certain keywords. Although the function's signaturekeyframesName :: String -> KeyframesName
didn't convey the notion of failure (e.g. through aMaybe KeyframesName
return type), my high-level plan was to eventually add some logic to make any invalidString
input into a validKeyframesName
value by changing the input in some way.Now I am thinking this would likely involve a complicated implementation, likely an imperfect one, and probably doesn't offer enough benefit to justify that cost.
For comparison, I also had a look at Halogen, whose
ClassName
type (moved to web-html in 2020) is analogous toKeyframesName
. Here's what I found:ClassName
constructor is exposed instead. See Web.HTML.Common.className
constructor function, but it was removed many years ago without explanation.className
function never included any validation or other logic to ensure that theClassName
return value would be a valid class name (e.g. free of whitespace). Perhaps this is why it was pruned.Thus, in removing
keyframesName
I am following suit with the leading PureScript UI framework, whose core maintainers' judgment I trust, even if they didn't document their rationale in this case.Moreover, I am considering (#29) adopting the
ClassName
andAttrName
types (from web-html, previously halogen). Since their constructors are exposed directly, I think the same pattern should be followed here for consistency's sake.Future plans
Revisit this issue if changes are made in
Web.HTML.Common
.References
<keyframes-name>
type<custom-ident>
typeCode change checklist