Possible to make dispatch type-safe?

[BartAdv]

Hello,

dispatch function currently works for every action type, and is needed to be called with unsafeCoerce, just like here: https://github.com/ethul/purescript-react-redux-example/blob/master/src/Example/App.purs#L118

Have you tried to approach this somehow, to push all the unsafe calls to library level, so that user won't have to be bothered with it (and possible, restrict the type of action as well)?

PS. I've experimented a bit with making the API more similar to original (that is, without introducing ReduxReactClass, just calling connect) - it would allow to have the (type-safe) callbacks in presentation components and then connect would do the unsafe magic, but seems it's not that easy. Namely, the library insist on having mapStateToProps separate from mapDispatchToProps, and I just don't know if it's possible to type something like this in Purescript ( (a->b) -> (d->e) -> merge b e)... was that the reason you've chosen the current way?

PS2. Is this library used somewhere? I found it pretty cool to be able to use the Redux tooling out-of-the-box, so I think the approach is very appealing.

[BartAdv]

On a second thought, stuff here is actually more nice than I thought initially (probably the example app doesn't show it best). I've currently ended up with something that following pictures:

https://gist.github.com/BartAdv/863140019fc6a806f8833b3ffad2dda5

As you see, no more unsafeCoerce when dispatching, the only unsafe thing is that redux components are not aware of the Action type so type system is not guarding you when you choose the action type to be dispatched.

[ethul]

Thanks for taking a look at the library!

I agree that it would be nice to have dispatch avoid the use of unsafeCoerceEff. The way you set it up in your example looks good, but maybe there is a way to do this at the library level too. I may have some time to work on this, but a PR is welcome on this.

Regarding the library's API, the goal is to provide a lens layer on top of redux and react-redux. I suppose it is more of an opinionated wrapper than a low-level one. I am open to discussing alternative ideas on this, but I think lenses provide a nice way to working with the different parts of redux.

And I am not sure if anyone else is using this library except me :)

Thanks for the link to the gist. The example looks good!

[BartAdv]

Thanks for the info. If everything goes well, I'm gonna use it too :)

[ethul]

Great!

…

On Sat, Jan 7, 2017 at 03:08 Bartosz ***@***.***> wrote: Thanks for the info. If everything goes well, I'm gonna use it too :) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAVYy57xtXxGrWLDBLS5kw2kIzG0RjmQks5rP0gXgaJpZM4LcSIj> .

[BartAdv]

Thought a bit about this:

What if, instead of feeding dispatch to component's lifecycle methods, we'd just pass the instance of a store down to every container component? Store is parameterized by a type of the action, therefore dispatch would operate on store and it'd be typed. Currently, if I don't annotate my render function, the dispatch signature will accept any action...

Basically, the idea that dispatch is in the props of a container component is react-redux idiosyncracy, I don't think it's something to build upon. This change is relatively simple, I'll probably tinker with it soon unless there's something obvious that I've missed.

The next step for better type safety would be to parameterize Middleware in a way that would allow type-safe composition (after all, next middleware may accept the different type of action):

type Middleware eff action action' state result = MiddlewareAPI eff action action' state result -> (action' -> Eff (ReduxEffect eff) action) -> action -> Eff (ReduxEffect eff) result

type MiddlewareAPI eff action action' state result = { getState :: Eff (ReduxEffect eff) state, dispatch :: action -> Eff (ReduxEffect eff) result }

Currently, each middleware can only call next with the very same action type....

[BartAdv]

After sleeping on it (the first issue, that is), I've realized it'd be quite a lots of (some unneeded, really) changes for that small benefit, however, how about parameterizing ReduxReactClass with action:

foreign import data ReduxReactClass :: * -> * -> * -> * -> *

type ReduxReactClass' state props action = ReduxReactClass state Unit props action

This affects the signature of createClass:

createClass :: forall eff f action props props' state state'. MonadEff (ReduxEffect eff) f => Getter' (Tuple state props) props' -> Spec props' state' eff f action -> ReduxReactClass state props props' action

This means, in my example I can just annotate the 'class' type of the container component for some extra safety, without the need to annotate render function (ReactElement is a boundary when the safety is lost anyway, but at least it simplifies some things for the class definition):

loginContainer :: ReactElement
loginContainer = Redux.createElement klass unit []
  where
    klass :: Redux.ReduxReactClass' AppState Login.State Action
    klass = Redux.createClass' Store.loginLens $ Redux.spec' render
    render dispatch this = do -- now its 'action' type is correctly inferred and fixed for `Action`, we can't feed `dispatch` with bogus action like we could previously
      props <- React.getProps this
      pure $ loginView { onClick: \({ mail, password }) -> void $ dispatch $ pure $ Login $ Login.AttemptLogin $ LoginData { username: mail, password }
                       , enabled: not props.inProgress && isNothing props.authData }

[ethul]

I think parameterizing the ReduxReactClass with action could be a good way to go. Though, I think I'd swap the ordering:

type ReduxReactClass' action state props = ReduxReactClass action state Unit props

To be consistent with the Enhancer and Middleware types, etc. Granted it is not consistent with the lifecycle methods, but I think that is okay.

[BartAdv]

Cool, I can open a PR for it, but since it's gonna be breaking change and, as you noticed, the order of parameters is off (and I must say I had to use 'goto definition' quite a lot when writing code, just because I didn't know whether the state param goes first, or props or whatever), maybe we could take a time and make it uniform? This would mean maybe it'd be better idea to follow the order used in lifecycle methods instead:

React bindings does this:

type Render props state eff =

So I guess we could make this a rule, and always have props before state:

type ReduxReactClass' props state action = ReduxReactClass props Unit state action

and action would come last, because it's an 'addition' to the basic functionality.

As for the types of enhancer/middleware, if I manage to roll their 'type-safe wrt composition' version, their signatures are gonna to be changed anyway.

[BartAdv]

Addendum: Middleware would most likely have to have action and action' as last type parameters if we'd like to make an instance of Semigroupoid (and have compose for it), so that's another reason to have actions at the end.

[ethul]

Sounds good to me to make everything uniform. I think whatever parameter order makes sense for how the type is used would be best (e.g., if we needed to partially apply it, etc.). Thanks for working on this PR!