-
Notifications
You must be signed in to change notification settings - Fork 95
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
Thoughts for 2.0 #171
Thoughts for 2.0 #171
Conversation
Amazing work, and fantastic base for 2.0. Give me a bit of time to digest and go through the changes you've made, but I think in general the direction is great. |
Great work! The API is so much cleaner now. There're a couple of issues though that have to be resolved before merging this PR.
@partheseas Is it ok if I make these changes on top of your work, or would you rather handle it yourself? |
@sameoldmadness Yeah, the intent was never for this PR to actually get merged, but more of "hey this is what I'm doing to simplify/improve things for me, feel free to copy if you like my ideas" so feel free to rebase and make whatever changes you want. |
Hi, it's me again! Hopefully you remember me from #145 😊
So I just wanna preface this by saying I don't expect this PR to get merged, but I did want to have a discussion!
I ended up really making myself at home on my fork while I worked on some stuff. A lot of changes in the diff are just boilerplate type things, but there's a lot of meat in there too.
Support for using Map and Set types in stores
Pretty simple, but adds a little machinery to serialize and deserialize Map and Set objects for initializing renderer state, like I had in my original PR
A greatly simplified API
That's it. That's all you need now. Renderer state initialization is now handled through an async action that we dispatch internally. Forwarding and replaying are handled from within the enhancer and don't need a separate function to subscribe to updates. It also doesn't need
applyMiddleware
. This was mostly out of necessity, since it doesn't provide the full store API, and I couldn't get the result I wanted without direct access to the store. It does have the nice side effect though of being just a little less code for the end user to type/understand. 🙂Cleaned up some internal workings
Particularly the way that the initial state is fetched for renderer processes really got cleaned up. Rather than using global variables and a getter function, it uses idc communication which allows it to also happen asynchronously. The renderer will start with the usual default state determined by Redux, and will be swapped out once the correct state has been fetched from main.
Particularly the API changes are pretty substantial, but I'm curious what you think about them!