onivim / oni

Oni: Modern Modal Editing - powered by Neovim
https://www.onivim.io
MIT License
11.36k stars 301 forks source link

Revising redux #2685

Open ghost opened 5 years ago

ghost commented 5 years ago

I'm afraid I'm going to make a lot of people angry by saying this, but I wonder: do we really need redux for every single component? I actually don't think we need it. Don't get me wrong: I love redux. I have used it many times before, and the time travelling is just awesome. But I wonder if all of the developer overhead is worth it. Because:

Granted, the state snapshots are useful for persisting state and for helping developers pinpoint a bug. But with so many different stores, this becomes even more difficult, if not impossible. Also the state of the Neovim editor depends entirely on an external program (nvim) in a one-directional way, so some of the benefits (like the time travel) are lost. We can't travel back in time because nvim will override every change!

What I Propose

It's easy to have criticism without providing some alternative. I think that issue #2684 actually could be able to solve many of the issues that Redux initially had to take care of, but I need to make sure. Using DI, in theory it should be possible to inject the NeovimEditor directly into the desired component and synchronize the state from there. This would also result in a slight performance improvement, because apart from an initial wiring step, signals coming from Neovim don't have to pass through a redux store before getting rendered.

Any thoughts on this one?

bryphe commented 5 years ago

Thanks for thinking about this, @samvv ! I'm definitely open to ideas / alternatives to Redux. We really used (maybe even abused) in some sense.

At least for me, I rarely used the time travel debugging in actuality, for a variety of reasons - either performance, or the bug wasn't quite captured in the state, etc.

We do certainly have a lot of Redux stores. I could see this pursued incrementally - we could identify components that don't really need their own store, and implement an alternative (sounds like that is the direction you're thinking!).

There are some cases too where we could replace our redux store with the Context API, too: https://reactjs.org/docs/context.html

Look forward to seeing what you come up with!

ghost commented 5 years ago

Thanks, @bryphe, for the positive feedback. I'll open a pull request integrating issue #2684 and this one into one. They are quite complementary.