Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

component re-mounts #105

Closed
coot opened this issue Jul 19, 2017 · 8 comments
Closed

component re-mounts #105

coot opened this issue Jul 19, 2017 · 8 comments

Comments

@coot
Copy link

coot commented Jul 19, 2017

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
Copy link
Contributor

ethul commented Jul 19, 2017 via email

@coot
Copy link
Author

coot commented Jul 19, 2017

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

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

@ethul
Copy link
Contributor

ethul commented Jul 19, 2017 via email

@coot
Copy link
Author

coot commented Jul 19, 2017

There's even shorter one :)

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

@ethul
Copy link
Contributor

ethul commented Jul 19, 2017 via email

@coot
Copy link
Author

coot commented Jul 20, 2017

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?

@coot coot mentioned this issue Aug 5, 2017
@ethul
Copy link
Contributor

ethul commented Oct 4, 2017

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
Copy link
Author

coot commented Oct 5, 2017

Yes, we should definitely document this.

ethul added a commit that referenced this issue Apr 14, 2018
Resolves #96
Resolves #105
Resolves #138
Resolves #139
ethul added a commit that referenced this issue Apr 14, 2018
Resolves #96
Resolves #105
Resolves #138
Resolves #139
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants