lukebarnard1 / matrix-redux-wrap

A library that exposes matrix-js-sdk state via Redux
Apache License 2.0
3 stars 1 forks source link

Thoughts on dog fooding attempt #1 #9

Open lukebarnard1 opened 6 years ago

lukebarnard1 commented 6 years ago

Having implemented a simple chat client built atop matrix-redux-wrap, I've realised a few things.

There is more to Async Redux than thunks

Enforcing redux-thunk limits the app developer. In reality there are a multitude of redux middlewares for handling side effects and mrw should accommodate as many as possible.

This means deprecating asyncAction, which assumes the redux-thunk middleware and suggesting any alternative. (FTR asyncAction has a similar API to redux-pack.)

Practically speaking, this means using some alternative in the examples (e.g. redux-pack) and not exporting asyncAction. But this is low priority.

The js-sdk emits too many events

By default, with a typical matrix account, too many events are emitted by the js-sdk. These would normally correspond 1-1 with actions dispatched by the wrapSyncingClient function, but mrw now exports an alternative that batches several actions into an array that are applied to a store in serial, blocking view updates on several sequential store updates.

The result is far from optimal, with the view "jumping" forward with the newly represented state.

This is a general short-coming of the architecture of mrw and mjssdk. It is a direct consequence of the matrix-js-sdk sync logic being ambivalent to the distinction between receiving a live event and loading a client that needs to catch-up to the current state of rooms.

The solution is unclear for now, and only impacts accounts with a high number of rooms, where the rooms have a high number of members.

Sharing mrw state with app state is confusing

Keeping mrw state in the same tree as app state is a strange experience. In my chat app, I had a single call to matrixReduce, followed by derivation of state from the mrw state.

Ideally, the state would already lend itself to being used directly, but use-cases will differ per client so I think it's safe to err on the side of simplicity in the library and allow for customisation of state in the app.

Given that reducers are typically composed by separating the state they work on, it would be interesting to consider keeping the matrixReduce reducer completely separate from the reducer that the app uses to maintain state outside of mrw. This would mean moving the state derivations out of the reducer entirely and into the view, which is not necessarily a more expensive thing, especially if the view is only rendered when absolutely necessary. This also has the advantage of less redundant state being stored and persisted.

huan commented 4 years ago

Hello luke, Thank you very much for your great project and idea-sharing!

I have a similar idea like yours that wrap all the events to Redux Actions for easy State management, I have created my own redux wrapper at Wechaty Redux, it's currently only an idea with some code from scratch at https://github.com/wechaty/HAWechaty/tree/1fdfa1a4cfe22cd0072c4955fd39bf64ad2062ae/src/ducks/wechaty

P.S. I'm the author of a Bot SDK for WeChat named Wechaty and a WeChat bridge for Matrix: Matrix Appservice Wechaty

lukebarnard1 commented 4 years ago

Hi @huan, I would be interested to see how much progress you can make there. I've wished for a long time that matrix-js-sdk could be written in a way that encapsulates matrix client state in a way that separates the event API and the OO-based APIs that may cause side-effects.

Redux seemed like a good solution to me at the time, but honestly the amount of boilerplate and the complexity of the loading/success/failure pattern that I wrote seems unnecessary in hind-sight. Similarly, wrapping every matrix-js-sdk event in a Redux action seems unnecessary given that the matrix-js-sdk already has a client with state.

The latest trend in React seems to be to encapsulate side-effects within a hook, and actually I think the matrix CS sync API could be effectively abstracted within a hook that accepts user credentials and other client options, and exposes all client state in a functional way.

To follow other libraries, this could be maintained within matrix-js-sdk itself, or in a separate library, but could use the existing functions of the SDK to expose a useMatrixClient or something similar.

huan commented 4 years ago

Hi @lukebarnard1, it's great to know that you are still interested in doing a full-redux-ed API version of the matrix-js-sdk!

I'd like to share some progress from mine in the past months: I'm trying to build a full-redux-ed module for my Chatbot SDK wechaty, with the ducks extension.

Currently, I have made a little progress for exploring the API by selectors/operators, here's a part of the source code of it in case you are interested.

And Wechaty SDK currently only supports WeChat protocol, however, I'm planning to build a Matrix Puppet for Wechaty, which can be very easy to enable Wechaty to work with Matrix bots.

If Wechaty begins supporting the Matrix protocol, then my wechaty-redux project will be able to work with the Matrix too.

Please feel free to let me know if you have any suggestions/questions, thank you very much!