purescript-contrib/purescript-react

component re-mounts

coot opened this issue · 8 comments

coot commented

This is not a bug per se, but I though it might be good to share. After all, I can write a pull request to include this somewhere in the documentation.

If you create a react class which is constrained:

inputSpec :: forall e a. Show a => ReactSpec (InputProps a e) Unit e
inputSpec = debugSpec $ ((spec unit renderFn) { displayName = "Input" })
  where
    handleChange this ev =
      let v :: String
          v = (unsafeCoerce ev).target.value
      in do
        { onUpdate } <- getProps this
        onUpdate v

    renderFn this = do
      { value } <- getProps this
      pure $ D.input [ P.value (show value), P.onChange (handleChange this) ] []


inputCls :: forall e a. Show a => ReactClass (InputProps a e)
inputCls = createClass inputSpec

Then it will be unmounted every time the onUpdate handler updates a state of the parent. The reason is quite simple. The generated code contains a function:

var inputCls = function (dictShow) {
    return React.createClass(inputSpec(dictShow));
};

So the react class is created dynamically, and hence on every render react will think that received a new class and it will unmount the current one and remount the new one. This loses focus on the input element - hence becomes unusable.

I stumbled upon this with slightly more sophisticated example where I wanted the type system to render the correct input element for a given purescript type, i.e. text for String, number for Int or Number etc...

ethul commented
coot commented

Fortunately, the PureScript compiler is clever enough so that this will work

inputCls' :: forall e. ReactClass (InputProps String e)
inputCls' = createClass inputSpec
ethul commented
coot commented

There's even shorter one :)

inputCls'' :: forall e. ReactClass (InputProps String e)
inputCls'' = inputCls
ethul commented
coot commented

It would be nice to have a warning, but I am not sure if that's possible. @paf31 any ideas? would it be useful elsewhere?

ethul commented

Would documentation suffice for resolving this? It might be something to make explicit in the readme if we end up using classes for more parts of the library.

coot commented

Yes, we should definitely document this.