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

Should we switch console app to use redux? #45

Closed
rundmt opened this issue Sep 20, 2018 · 7 comments
Closed

Should we switch console app to use redux? #45

rundmt opened this issue Sep 20, 2018 · 7 comments
Labels
question Further information is requested

Comments

@rundmt
Copy link
Contributor

rundmt commented Sep 20, 2018

As we're growing the app we want more functionality. Custom colors, fonts, saving user data. Would now be a good time to bring in redux? Currently we're just passing props everywhere, but we probably need some sort of app state management that can handle things like deeper nested panels.

Is the console app small enough that it doesn't matter? Or should we implement something like redux? Also we could use context?

@rundmt rundmt added the question Further information is requested label Sep 20, 2018
@ryanjduffy
Copy link
Contributor

Would be a good opportunity to see explore a couple different state management methods/libraries

@rundmt
Copy link
Contributor Author

rundmt commented Sep 20, 2018

Makes sense. I was thinking about RXJS mostly because AGL uses websockets to communicate. That might be a more ideal way of dealing with them compared to redux.

@webOS101
Copy link
Contributor

@rundmt
Copy link
Contributor Author

rundmt commented Sep 21, 2018

So the first investigation @haileyr and I ran was using context API. We're also using it with immer.js so remember to npm install if you checkout this branch. I wanna focus on 2 interesting things. So using the context API we can pass up the state to the top level, and using immer.js we can get shared immutable data structures but still have a very simple API that looks like we're just mutating an object.

Here's some lines that I think can be interesting.

With this line we can update the AppState using immer's produce function. It just takes in a callback similar to react's functional setState. https://github.com/mweststrate/immer#reactsetstate-example. We can use this to change anything in our state.

feature/customization-screen...feature/immer-context#diff-c3d9078ea82f2329d2caad4cc872e97dR152

Here we have our consumer pulling in the updateAppState from context. This can now be updated and passed back up the tree very easily just by mutating the object. No need for fancy spread operators or Immutable.js.

feature/customization-screen...feature/immer-context#diff-0bf03b8790698ab728e61962b79fcb6dR13

This is a pretty naive approach for performance but maybe with a little tweaking we can get the benefits of a simple way to update an App and we can harness the power of immutability for React performance. We could possibly make smaller contexts instead of passing down the whole app. We could also do some testing to see if re-render time is not too bad when using PureComponents.

@rundmt
Copy link
Contributor Author

rundmt commented Sep 24, 2018

So while implementing mobx I'm running into some weird stuff where for some reason it doesn't work in Enact. I made a create-react-app to compare and updates happen just fine in the create-react-app version, but don't update in even the most barebones Enact app. I'm not using kinds or anything. The code is literally duplicated and it doesn't work in our stuff. I'm not sure if our build stuff is missing something, but something is preventing it from updating.

Here is the most basic version of the app if anyone else wants to test. There is a possibility that this could be my machine only. The text should update while typing.

import React from 'react';
import ReactDOM from 'react-dom';
import { observable, computed, action, decorate } from "mobx";
import { inject, observer, Provider } from "mobx-react";

const App = inject(["store"])(
	observer(
		class Input extends React.Component {
			render(){
				console.log('render');
				console.log(this.props.store.onChange)

				return (
					<div>
							<p>Display</p>
							<p>{this.props.store.text}</p>
							<input type="text" onChange={this.props.store.onChange}/>
					</div>
				)
			}
		}
));

class State {
    text = ""; // observable state
    onChange = e => {
      // action
      this.text = e.target.value;
    };
}

decorate(State, { text: observable });

const appState = new State();

ReactDOM.render(
    <Provider store={appState}>
        <App />
    </Provider>,
    document.getElementById('root')
);

@rundmt
Copy link
Contributor Author

rundmt commented Sep 26, 2018

@sugardave @haileyr and I discussed somethings on our findings. We think using something like the context API is probably the easiest way to go. It's pretty straightforward and simple. It is much less boilerplate than something like redux. We think this is a plus for developers. If for some reason we do need to deal with subscribing to anything like a socket it can be the developers choice to bring in something like RXjs if they wish to have that level of control.

RXJS is simple to use, but we found the documentation confusing. Also there are not a lot of examples of using RXjs with React which would probably lead to more problems down the road.

Mobx works fine, but using something like mobx is basically like using context, but you have to add a bit more boilerplate. It works well, and it can be organized, but we think context API seems like the way to go for the future. Also the docs on Mobx get tough if you don't use decorators or you want to switch to mobx-state-tree. It makes searching for examples tougher.

I'll do a deeper dive into performance of the context API to see if it can be more performant, but i think using something like context over redux or mobx makes sense just because of how easy it is to get started.

@ryanjduffy
Copy link
Contributor

@rundmt - will you be opening a PR for the console app with this change to use the Context API? I'd love to see it in action across the entire app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants