Add support for createRef

[elliotdavies]

I was doing some exploratory work around adding support for React's createRef API when I noticed a lot of similar work had already been done as part of the hooks PR: https://github.com/purescript-contrib/purescript-react/pull/159/files#diff-d96e307e87fe77fec439e08ec13d4947

I'm not sure what the status of that PR is, but how do we feel about introducing the createRef part of it separately? (Probably including the new Ref a type, the accompanying functions, and the ForwardRef stuff.) I'd certainly be keen to help with this.

[ethul]

Thanks for looking into the Ref functionality. If you're interested in creating a PR with just the createRef API separately, I think that would be great! Feel free to use anything from #159 if that helps.

[elliotdavies]

After looking into this over the last couple of days, I'm struggling to find a pleasant implementation that supports both the new createRef API as well as the old callback style. Below is a problem summary, a few of the approaches I've tried, and then some thoughts on how to proceed. It'd be useful to get some feedback before I make a call on it.

Problem summary

There are two ways to create refs (createRef and callbacks) and two places that refs can be passed (DOM elements and React classes), so we have four cases. In pseudo-Purescript:

myClass :: React.ReactClass {}
myClass = React.component "MyClass" \this -> do
  domRef <- Ref.createDOMRef
  instanceRef <- Ref.createInstanceRef

  pure
    { state: { domRef, instanceRef }
    , render: render this <$> React.getState this
    }
  where
    render this { domRef, instanceRef }
      = React.DOM.div
          []

          -- 1. DOM ref using createRef
          [ React.DOM.input
              [ Props._type "text"
              , Props.ref domRef
              ]
          
          -- 2. DOM ref using callback
          , React.DOM.div
              [ Props.ref \ref -> do
                  -- Do something here
              ]
              [ ]
          
          -- 3. Instance ref using createRef
          , React.createLeafElement anotherClass
              { ref: instanceRef
              }

          -- 4. Instance ref using callback
          , React.createLeafElement anotherClass
              { ref: \ref -> do
                  -- Do something here
              }
          ]

The challenge therefore is to find a nice API that works in all these cases.

It's also worth noting that in React you get slightly different results depending on how you create a ref: with createRef you'll get something like { current: <ref> } but with a callback you'll get <ref> directly. I generally assumed we'd want to keep this distinction.

Attempted approaches

Ref a

I started with an approach along the same lines as @ethul tried in the hooks PR here: https://github.com/purescript-contrib/purescript-react/pull/159/files?file-filters%5B%5D=.js&file-filters%5B%5D=.purs#diff-039a468442c8f8ed540e0e4941930e29

This setup works really well for the first three cases, provided we don't mind having separate Props.ref and Props.callbackRef functions for DOM elements (I think this is fine). However, it doesn't immediately work for props passed to React classes since either ref needs to be one of two distinct types or we need to support a new reserved prop called callbackRef. So, I tried...

Adding a reserved callbackRef prop

This approach would give us something like:

type ReservedReactPropFields r =
  ( key         :: String
  , ref         :: Ref ReactInstance
  , callbackRef :: SyntheticEventHandler (Nullable ReactInstance)
  | r
  )

I think this is a misleading type since it suggests to the user that they can pass both ref and callbackRef at once, which shouldn't be supported. Additionally we'd have to put in some code to intercept the callbackRef prop and rename it to ref before it gets to React, and it all gets very messy.

Type classes

I really wanted to be able to write something like

type ReservedReactPropFields ref r =
  ( key :: String
  , ref :: ref
  | r
  )

and then elsewhere add a constraint like IsReactRef ref => ... and make the relevant types (e.g. Ref ReactInstance and Ref (Nullable ReactInstance -> Effect Unit)) instances of this class. Sadly neither I nor @LiamGoodacre could get this to work at all.

Sum types

Finally I tried just using a sum type to represent the possibilities. For example:

data InstanceRef
  = InstanceRefObject ReactInstance
  | InstanceRefCallback (Nullable ReactInstance -> Effect Unit)

To do this we'd have to add helper functions to be used like so:

-- 3. Instance ref using createRef
, React.createLeafElement anotherClass
  { ref: Ref.withRef instanceRef
  }

-- 4. Instance ref using callback
, React.createLeafElement anotherClass
  { ref: Ref.withCallbackRef \ref -> do
    -- Do something here
  }

Even if we do this, the ref prop being part of a row means I can't see any good way to unwrap the type later so that the inner ref or callback can be passed to React. (I looked at Record.modify etc but the field is optional and so I don't think it's possible to modify it only if it exists? At any rate, we'd still have to repeat that on createElement, createLeafElement and all the other similar functions.)

Proposed solutions

I've very possibly missed some type-level trickery that might help resolve this problem. Or, at the other end of the spectrum, I could move more of this logic into the FFI (for example, I could do the InstanceRef sum type unwrapping on the JS side). This seems it should be a last resort though!

Alternatively we could:

• do nothing - but I think we ought to keep pace with React
• drop support for callbacks and move wholly to createRef - but that would be very opinionated
• use the initial API design but support only the first three cases (i.e. not support callback refs for React classes) - but that'd be a weird middle ground and I'd really like to support all of the options

The only other thing I can think of would be to add new functions like createElementWithRef and createLeafElementWithRef and just pass the ref as a function argument; at that point I think we could use an IsReactRef typeclass without issue. This would be a fairly big departure from the current row type implementation, though; and we might then want to reconsider how reserved props are treated in general.

Thoughts most welcome 😄

[natefaubion]

In my local projects when dealing with FFI overloading like this, I usually use an opaque foreign data type with smart constructors that are coercions under the hood. Maybe something like:

foreign import data RefHandler :: Type -> Type

foreign import fromEffect :: forall a. (a -> Effect Unit) -> RefHandler a
foreign import fromEffectFn :: (EffectFn1 a Unit) -> RefHandler a
foreign import fromRef :: Ref a ->RefHandler a

[ethul]

@elliotdavies Thanks for working out the above problem summary and proposed solutions. It is very much appreciated. I am inclined to agree with @natefaubion that using the opaque foreign type along with functions for the appropriate coercions is a good way to go. Would you be open to trying something like this?

[elliotdavies]

@natefaubion @ethul Cheers for getting back to me. This approach sounds like it should work - I'll certainly give it a go. I'm away for the next three weeks but I'll see about it once I'm back 😄

[elliotdavies]

@natefaubion @ethul Sorry for the lengthy delay! I finally got round to this and have submitted a PR for discussion.