search

liqula / react-hs

A GHCJS binding to React based on the Flux design. The flux design pushes state and complicated logic out of the view, allowing the rendering functions and event handlers to be pure Haskell functions.
32 stars 10 forks source link

Fix type of img_ (and any other childless elements) #64

Closed dbaynard closed 6 years ago

dbaynard commented 7 years ago

img_ as implemented requires a child element, but unless that element is mempty there is a runtime error.

This is mentioned on the react-flux issue tracker; the proposed fix is to replace 

img_ :: Term eventHandler arg result => arg -> result
img_ = term "img"

with

img_ :: [PropertyOrHandler eventHandler] -> ReactElementM eventHandler ()
img_ p = el "img" p mempty

but

  1. it is a breaking change,
  2. there may be other affected elements (which might as well be fixed), and
  3. it still is not as correct as it could be in the sense that (at least according to Mozilla the src is mandatory, yet that is not reflected in the type.

So I propose

  1. Gather a list of elements
  2. Change them all before the first hackage release (code changes are trivial), and
  3. Consider a broader way of typechecking required properties (perhaps via a typeclass constraint such as Required "src") as a separate issue, if desired.

See https://bitbucket.org/wuzzeb/react-flux/issues/27/dom-element-img_ for original issue and solution.

fisx commented 7 years ago

good analysis, good plan!

This is not a high priority for us, but we would welcome PRs. The breaking change is benign IMO, since the type checker will catch it.

dbaynard commented 7 years ago

The elements. Some use term' or term inappropriately, others manually implement the same repeated pattern.

Elements Wrongly requires child Manual implementation
area ✓
base ✓
br ✓
col ✓
embed ✓
hr ✓
img ✓
input ✓
link ✓
meta ✓
param ✓
source ✓
track ✓
wbr ✓
tysonzero commented 7 years ago

I think in the long run we should essentially replicate the HTML spec in the type system (and then perhaps have a few unsafe functions that can bypass it, lets say if you are targetting some quirky older browser). So requiring required attributes and only allowing attributes that can actually be used. Even going as far as having ADTs for each CSS property and stuff like that would IMO be very cool. But not super high priority. Perhaps if others agree with me I could look into doing such a thing if I have enough free time.