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

[RFC] New dependency: Easy Peasy State Management #3442

Closed
2 tasks done
damassi opened this issue Jun 11, 2020 · 25 comments
Closed
2 tasks done

[RFC] New dependency: Easy Peasy State Management #3442

damassi opened this issue Jun 11, 2020 · 25 comments
Assignees
Labels

Comments

@damassi
Copy link
Member

damassi commented Jun 11, 2020

New Dependency

Name: easy-peasy
URL: https://github.com/ctrlplusb/easy-peasy

Focus

Easy Peasy is a popular state management solution built on battle tested redux. It provides a modern API and comes with everything one needs to quickly manage inter-component state without needing to write everything from scratch. It has first class TypeScript support, gives us access to the redux middleware system, comes with an immer-based state update patterns, and proves some excellent (and simple) abstractions around observing actions within a system and acting on them without having to write mountains of glue code.

Example from docs:

const store = createStore({
  todos: {
    items: ['Create store', 'Wrap application', 'Use store'],
    add: action((state, payload) => {
      state.items.push(payload)
    })
  }
});

function App() {
  return (
    <StoreProvider store={store}>
      <TodoList />
    </StoreProvider>
  );
}

function TodoList() {
  const todos = useStoreState(state => state.todos.items)
  const add = useStoreActions(actions => actions.todos.add)

  return (
    <div>
      {todos.map((todo, idx) => <div key={idx}>{todo}</div>)}
      <AddTodo onAdd={add} />
    </div>
  )
}

Going forward Eigen would benefit from a consistent, hooks-based batteries-included state management pattern.

Check List

  • Have read over the source code, and we can maintain it
  • Has had a release in the last year, or looks done and stable

Alternatives

There are quite a few alternatives in the react state management space. We could use redux directly, but the common complaint is that it's too verbose. We could also use react's context directly, but that would mean a lack of consistency, needing to hand-roll perf optimizations, no middleware, etc.

@damassi damassi added the RFC label Jun 11, 2020
@damassi damassi changed the title RFC: Easy Peasy State Management [RFC] New dependency: Easy Peasy State Management Jun 11, 2020
@pvinis
Copy link
Contributor

pvinis commented Jun 11, 2020

I don't know if this was discussed or needed or whatever, but I think it's nice to keep our tools fresh. Besides easy-peasy, I've used https://redux-toolkit.js.org/api/createSlice which is a breath of fresh air after "regular" redux. It makes handling redux way easier, so I'd like to nominate that as one of the alternatives, but why do we need redux?

@damassi
Copy link
Member Author

damassi commented Jun 11, 2020

Why do we need redux?

Because we have a sophisitcated application, and rather than rewriting state managment every time we need manage state we can look at things that the team has worked with in the past, things that have a vibrant, healthy ecosystem and a good track record at scale.

However, I would like to focus the question on: why easy-peasy? That's the point of this RFC. There are plenty of "why redux" articles online and some inherent bias I'd like to combat. The end user doesn't even need to know that redux is below, unless they would like to tap into the middleware ecosystem and add something like 1-line-of-code localStorage persistence, for example. Dive into the easy-peasy docs to learn more.

More specifically to product needs, this came up in response to a pretty massive app we're about to undertake on consignments that has a lot of global state, and we have a good opportunity to establish some patterns here:

Screen Shot 2020-06-11 at 10 09 10 AM

@jonallured
Copy link
Member

Something that's unclear to me is the relationship between state and routes. My feeling is that Formik can handle the state on the forms and routes can implicitly handle the state of which thing to show. I'm still struggling to see which bits of state easy-peasy will help with.

@damassi
Copy link
Member Author

damassi commented Jun 11, 2020

Formik will indeed handle form state, but we will need to coordinate things from a higher point of view; for example, locking and unlocking route navigation, saving and restoring form state, and in general thinking about things from a global top down perspective via an orderly action-based flow that genericises execution across a very wide component tree. Sophisticated user flows in particular benefit from top-down thinking.

In addition, this is an opportunity to establish some state patterns going forward for other work in Eigen (and elsewhere) in an attempt to coral some our custom React.createContext usage in favor of something that's simpler, consistent and also more performant.

@pepopowitz
Copy link
Contributor

opportunity to establish some state patterns going forward for other work

Is the idea of this proposal that we would use easy-peasy for all state management moving forward in eigen, or just in complex cases? I still believe that the built-in react solutions for state management are suitable for 90% of cases, especially react Context.

There is complexity in redux-based solutions, and a learning curve for the team - it's a different way of thinking. The benefits it brings are not free. Having managed apps that were built entirely on redux for state management, my experience was that the amazing debugg-ability you get from redux tools was often only necessary because of the fact that state management had become so much more complex.

@damassi
Copy link
Member Author

damassi commented Jun 11, 2020

@pepopowitz - This RFC is for easy-peasy to be added as a dependency. There is inherent bias around Redux that I would like to avoid as mentioned above. Please see easy-peasy docs and an example implementation in the description of the RFC. Your experience with redux may have been less than satisfactory in the past, but we're talking about a different abstraction with a low-to-no learning curve for most features. If there are concerns about easy-peasy's design choices or where it might not work this is the place to discuss!

@ashfurrow
Copy link
Contributor

Thanks for the details Chris. I think using easy-peasy makes sense in the abstract, though I do share some questions from Jon about how specifically it would be helpful. Eigen doesn't really have a lot of global state to manage in between most screens; while this new Sell app might be an exception, adding any dependency beyond React would increase the barrier to entry for engineers contributing to that code. As Steve said, these abstractions do have a cost, and I want to be sensitive to it in the long run.

To help me understand where easy-peasy will help us, could you give an example from the Figma designs where easy-peasy would help manage state? I know React Context isn't an ideal API, but I'm curious about comparing how it would work in that example.

@jonallured
Copy link
Member

I guess where I'm landing on this is that I'm not convinced we have a situation where the use of easy peasy is warranted. I'm not saying that easy peasy isn't a great lib, just that while spiking and tinkering with this project I have not seen problems for which it is required.

@damassi
Copy link
Member Author

damassi commented Jun 11, 2020

Eigen doesn't really have a lot of global state to manage in between most screens

This isn't about just managing state across screens, its about intra-component-tree communication. Doing a search in Eigen, there's twelve uses of createContext, and thus twelve separate API's for managing global component tree state. If that were unified behind a single library / pattern (and it easily could be) then the complexity level would be reduced, app maintainability would increase, as would app performance.

I know React Context isn't an ideal API

It's not that its not ideal, it's that it's a primitive that provides no API requiring devs to create their own each time its used, which in turn increases complexity across the app. It also requires that we manage performance on our own, versus intelligent (invisible) optimizations built in at the library level.

could you give an example from the Figma designs where easy-peasy would help manage state

It's about orderly control flow. Mentioned above:

and in general thinking about things from a global top down perspective via an orderly action-based flow that genericises execution across a very wide component tree. Sophisticated user flows in particular benefit from top-down thinking.

Is the form complete? Check. Navigation is unlocked. Back button press? Are you sure? Dispatch action to show confirm? Confirm. Dispatch action to navigate. I can continue elaborating. All of these things sit centralized in one location and conform to a consistent API outside of the component callee. This improves long-term maintainabiliity. This stuff could be moved into useReducer, but that's a raw primitive that is more complex and less performant than an easy-peasy store.

Additionally, easy-peasy allows for the ability to listen for events being dispatched within the system. For example: user has created a new artwork. The artwork list view then receives a notification to perform a relay refetch after our navigation system, which was also listenting for this event, transitions to the artwork list view page. All of these abstractions and patterns -- which we'll have to write from scratch -- come for free with easy-peasy, in a very good / simple API.

@ashfurrow
Copy link
Contributor

Okay, thanks for elaborating.

I'm just trying to make sure I understand the trade-offs here. We have had a conservative policy around adding new iOS dependencies for several years and it's helped us prevent what happened in the native side of the app: multiple dependencies to do the exact same thing, because the team didn't communicate&socialize the benefits/capabilities of new dependencies.

I'm not saying easy-peasy is a bad solution – it sounds like a nice library, actually! I just want to make sure that I understand its value and tradeoffs because my team will be responsible for it in the long term. Am I making sense?

@damassi
Copy link
Member Author

damassi commented Jun 11, 2020

That makes sense!

When we write createContext, instantly there's a need to a) write boilerplate; and b) write correct types; and c) manage performance manually. By introducing a formal "batteries included" state management solution into Eigen we get a few wins in a few different dimensions, even for small things.

If it helps, after this My Collections work calms I'd be happy to volunteer some migration work around our existing usage, too, and further evangelize. @ds300 might have some thoughts here as well.

@ds300
Copy link
Contributor

ds300 commented Jun 12, 2020

Aside from this new Consignments work, we'll need to implement a global app state for auth, lab options, etc, as soon as we take the plunge on Android in Q3. I think easy-peasy would be a perfect fit for that. Its 'batteries-included redux' approach seems great to me. It solves most of the problems I had with redux back in 2016-2017 and then some, and I'm confident it would provide a very robust foundation for building state-heavy features going forward.

I see no reason not to adopt it. In retrospect I kinda wish I had suggested it to @ashleyjelks back when she started working on the collection filters since she ended up having to reinvent certain patterns not provided by useReducer that easy-peasy makes... well... easy-peasy.

@pvinis
Copy link
Contributor

pvinis commented Jun 12, 2020

I think it will be a good addition, and like you say Chris. My initial concern was I guess me being afraid of having it "too easy" to just use easy-peasy/redux for all kinds of state management. I think with relay, things are pretty alright. I agree the Context parts would become easier, I'm all for it. I just want to make sure or be clear that we will not start mixing these two willy-nilly until we end up with a tangled mess of data living in random places and getting fetched from all over the place, and having to manage that.

If we stick to using easy-peasy/redux for the purposes of managing state, as in flows, (navigation, notifications, consistent and common api for all our stuff, basically something more like a state machine) rather than state, as in data, (fetched data etc) then merge this already 😅.

@ashfurrow
Copy link
Contributor

Okay, thanks for the discussion so far everyone! Here's a recap of what I have learned:

  • Eigen could already benefit from a better way to manage global state than React Context, and this need will only increase over time.
  • easy-peasy seems like a good solution for managing global state.
  • For the immediate need, Consignments in specific could benefit from this solution.

Therefore, I think:

  • We need to answer @pepopowitz's question: is easy-peasy is the go-to solution for all state management in Eigen, or if it should be just for complex state? I'm curious to hear what others think. We should document our decision here.
  • We can accept this RFC. I'm satisfied this makes sense, but I'm still open to hearing from those who disagree.
  • To help facilitate this transition, we should convert (at least some of) the existing uses of createContext to use easy-peasy. Ideally in a group, maybe a Knowledge Share or a Lunch & Learn. It's really important to me that we socialize this change and make sure everyone is supported here. @ds300 do you think that the filters would be a good place to start, or should we begin smaller?

Are there any other questions/concerns anyone has? This is the exact right time to bring them up 🙌

@pvinis
Copy link
Contributor

pvinis commented Jun 12, 2020

* To help facilitate this transition, we should convert (at least some of) the existing uses of `createContext` to use easy-peasy. Ideally in a group

+10000000 (also, I'd like to be there too)

@jonallured
Copy link
Member

jonallured commented Jun 12, 2020

Some further discussion of how easy-peasy vs another approach compare here: #3451 (comment).

@damassi
Copy link
Member Author

damassi commented Jun 12, 2020

@ds300 do you think that the filters would be a good place to start, or should we begin smaller?

I was thinking the same thing, that that's a great first place. I think the LOC count will nicely condense too.

@ds300
Copy link
Contributor

ds300 commented Jun 12, 2020

agreed. refactoring the filters would be a great place to start since the architecture is so similar!

@sweir27
Copy link
Collaborator

sweir27 commented Jun 12, 2020

As I was reading this I was also going to suggest the filters as a place that could have benefitted from a library like easy-peasy! Happy to help out with that migration, sounds fun. 😄

I don't have too much to add but it will certainly be nice to have a preference when the need does arise (which it certainly will) for more complex state management. Easy-peasy seems like a good choice!

@damassi
Copy link
Member Author

damassi commented Jun 12, 2020

is easy-peasy is the go-to solution for all state management in Eigen

In most instances useState will suffice. But if one finds themself spinning up a new context, or writing a reducer that's shared across a handfull of components, using easy peasy will simplify ones code and eliminate all the boilerplate.

@ashfurrow
Copy link
Contributor

Right, I should have specified shared state. That makes sense to me. Are there any circumstances that we might want to use contexts instead?

@damassi
Copy link
Member Author

damassi commented Jun 12, 2020

I would imagine a context being used in a situation that may grow into a library that can be extracted from eigen where we'd want to keep the external dependency count low. (Though even in that case, personally, I'd rather use a proper state management solution than roll my own.)

Mentioned above, every time React.useContext is written it means a dev is creating their own custom API for shared state, because useContext doesn't come with an API. Unless there's some truly crazy unexpected thinking involved, the API uniquely created for that useContext call will look very, very similar to what we'd get out of the box with easy-peasy, only more complicated and requiring more lines of code with less amenities. For maintainability through time, less is more.

All of this is to say, in my ideal gate-keeper world I would see createContext as tech debt.

@damassi
Copy link
Member Author

damassi commented Jun 12, 2020

@pvinis

I just want to make sure or be clear that we will not start mixing these two willy-nilly until we end up with a tangled mess of data living in random places and getting fetched from all over the place, and having to manage that.

Very much agree 👍 Damon mentioned at his last gig they were storing their Apollo store in Redux creating a real mess of things. That's an architectural design problem that could manifest in any number of ways, and has nothing to do with shared state management in principle. Relay is Relay and shared state is shared state. (And in any case, Relay has its own global context / store that one can acess from anywhere.) It will be up to the team to catch issues like this in code review.

@damassi
Copy link
Member Author

damassi commented Jun 15, 2020

Closing with Adopted.

We'll be using this first in the incoming MyCollection Eigen app and then migrating collection filters over to the new setup.

Thanks everyone for your input 👍

@damassi damassi closed this as completed Jun 15, 2020
@damassi
Copy link
Member Author

damassi commented Jun 18, 2020

Resolution

We decided to do it.

Level of Support

3: Majority acceptance, with conflicting feedback.

Additional Context:

Some people were in favor of it, but others felt that a library that used redux might be overkill.

Next Steps

Being implemented in the new MyCollection app in Eigen. Collection filters will be refactored.

Exceptions

No exceptions.

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

No branches or pull requests

8 participants