Encoding optional props with ReactPropFields

[jvliwanag]

On the current master, createLeafElement has the signature:

-- | Create an element from a React class that does not require children.
createLeafElement :: forall required given.
  ReactPropFields required given =>
  ReactClass { | required } ->
  { | given } ->
  ReactElement

I used to be able to use Union for encoding react classes with optional prop fields defined such as:

foreign import fooClass :: forall props. ReactClass props

foo :: forall o t
           . Union o t ( title :: String )
           => {|o}
           -> ReactElement
foo = createLeafElement fooClass

Wherein I can call foo {} or foo {title: "hello"}. But it seems the new ReactPropFields constraint now prevents me from doing this. Any suggested way getting around this?

[ethul]

Out of curiosity, which version or commit was this working on?

@natefaubion any thoughts?

[jvliwanag]

Union approach works properly v5.1.0 with:

foreign import createElement :: forall props.
  ReactClass props -> props -> Array ReactElement -> ReactElement

I'm currently trying to upgrade doolse/purescript-reactnative which uses the Union approach for optional props.

[natefaubion]

The type of ReactPropFields and createElement is determined by the type of the ReactClass provided to it, however you have forall props. ReactClass props. This type would need to be instantiated to something. There is no way to safely express what you want with createElement because there's no way to verify that forall props. adheres to Reacts special cases around props, since by definition we don't know what it is.

[jvliwanag]

If that's the case, how should I define fooClass and foo if I want both of the ff to work?

• foo { title: "hello" }
• foo {}

[natefaubion]

PureScript doesn’t have optional fields, so what you’d do is use Record.merge with some set of default props before passing it in. Otherwise I would import fooClass as ReactClass {} instead and use unsafeCoerce in your smart constructor.

[jvliwanag]

Hm, I guess I've been wanting Option 1 as discussed in #147. This has worked nicely in purescript-reactnative with purescript-react 5.1.0.

My problem with the default props is that the defaults are not always known esp for foreign react components. I'll probably just try out unsafeCoerce for now.

Alternatively, why not have two row types for ReactClass? One for required as it stands now, and another for optional. Then the getProps for entries in optional could be wrapped in Maybe. The createElement should then accept a single record composed having all required rows and possibly optional rows.

[natefaubion]

My problem with the default props is that the defaults are not always known esp for foreign react components.

What do you mean by this? Do you mean something like pass-through props? You can construct a ReactClass with polymorphic props, but you must instantiate it to a monomorphic type when using it based on the props you are giving it.

Alternatively, why not have two row types for ReactClass? One for required as it stands now, and another for optional. Then the getProps for entries in optional could be wrapped in Maybe. The createElement should then accept a single record composed having all required rows and possibly optional rows.

This is an orthogonal issue to what the constraints are trying to accomplish. Right now it's impossible to use createElement to construct something with a key or ref. That's because key and ref are not part of the ReactClass specification, but rather part of the interface to createElement, yet React requires you to mix these things together in the same bag of props, which is very unfortunate since it is so awkward to type. I'm happy to consider keeping the old signature, and exporting this as an alternative. I've also opened https://github.com/purescript-contrib/purescript-react/issues/152 to provide an alternative which does not use createElement at all and is easier for us to type.

[jvliwanag]

Default props - when integrating third party components, with lots of optional props (say https://facebook.github.io/react-native/docs/view.html#props), it is:

1 - tedious to find out which default each prop should take 2 - sometimes (1) is not really practical since the default is most likely null/undefined 3 - (2) would then end up with ugly type signatures (full of Nullable).

As such a Union based API is just much more friendly to work with. I understand that ReactPropFields is used just to introduce key and ref props. The issue is that it prevents from a Union based API to work properly. That said #152 does seem to provide a nice alternative.

--

If we want to embrace optional fields though, and we're open to change the ReactClass definition, why not have explicit sections for required and optional props? Meaning:

foreign import data ReactClass :: # Type -> # Type -> Type

button :: ReactClass (label :: String, onPress :: Int) (border :: Int)
button = ...

mkButtonElement1 =
  createElement button { label: "Hello"
                       , onPress: 5
                       , border: 5
                       }

mkButtonRequiredOnly =
  createElement button { label: "Hello"
                       , onPress: 5
                       }

See gist for full example.

Imo, this definition would mirror how most JS defined react components are used. When working with purescript react components, we can maybe change getProps to emit a record containing all required props and wrapping all optional props with Maybe.

I can work on this if this is an acceptable approach.

[ethul]

Regarding integrating with third-party components, I think I prefer the implementation mentioned in Option 2 - Builder in #147, albeit more time consuming to write. The main reason is that I think it is handy for defining props where you might want a more complex data type for the prop value. In the React Native View example, if you wanted data PointerEvents = Auto | None | BoxNone | BoxOnly, mapping that to a string could be done in the builder function for the pointerEvents prop. I do think Option 1 is more convenient, but I think Option 2 is more flexible with respect to data types.

[natefaubion]

Imo, this definition would mirror how most JS defined react components are used. When working with purescript react components, we can maybe change getProps to emit a record containing all required props and wrapping all optional props with Maybe.

I don't know how to do this without significant overhead. A single type for props reflects the actual React interface. As far as implementation goes defaultProps is part of the createElement interface, and in reality all it does is merge prop records, which is exactly what I recommended above. Trying to hook into getProps make everything much more complicated and error prone. There is no reason you can't define a separate ReactDefaultClass as you've stated with an appropriate constructor to ReactElement, and it would still absolutely be compatible with this library.

[jvliwanag]

I'll continue to experiment approaches for this, but for now, the unsafe variants on createElement that's part of #149 should do. Thanks!