React Context happy fun times

[smashwilson]

So we pass a lot of props around.

https://github.com/atom/github/blob/fbaf0d4e10eaf718907570568d2a659158ba5fc6/lib/controllers/git-tab-controller.js#L124-L174

React 16.3 introduced a fancy new Context API which doesn't even carry the "please don't use this" scare language that the old one had. We can likely take advantage of this to make our prop situation much more manageable, especially for accessing:

• Atom environment properties
• Synchronously available Repository data
• Bound action methods

This isn't a trivial implementation, though. There are a lot of weird edge cases in the ways we handle event subscriptions, re-rendering timing, and so on that make this tricky to implement in a way that's a clear improvement. We would also have to be careful not to just re-implement Redux poorly.

[smashwilson]

@annthurium I'm guessing that this is our path to dealing with our prop drilling problem. 🤔

I see three possible paths forward here:

1. Incrementally extract sets of related drilled props to a context-based model. The Atom environment would be a prime candidate, for example. This will be easier for some than others; in particular, the Repository will be a challenge, because everything we care about in a Repository is fetched asynchronously in a container, and we'd need to be especially careful not to regress performance by re-rendering large swathes of the component tree unnecessarily.
2. Lift all state up to the root. Implement some higher-order components to manage an individual component's state change subscriptions. Lift all actions up to the root and operate (synchronously or asynchronously) on the state tree. Essentially: Roll our own Redux-ish architecture. We may be able to make some different assumptions and avoid some of the complexity that Redux introduces... or we might just reinvent the wheel and slowly re-learn things that Redux already learned years ago.
3. Go full Redux 🚀 by... actually using Redux. We've avoided this in the past because some of our usage patterns run counter to the way that Redux prefers to do things (@binarymuse would know details there, although it's been a while so I'm sure her knowledge is rusty :wink:). I'm also not sure how nicely it would play with Relay.

What do you think? Were there other alternatives that you had in mind that we could pursue?

[annthurium]

I was thinking something along the lines of 1) because that seems the most amenable to incremental refactoring.

I .. I don't love Redux because I feel like the functional language terminology it uses can be confusing, even though at its heart Redux is really just fancy pubsub. But just because I don't love it doesn't mean we shouldn't use it. What are the relative advantages of using Redux instead of the context api here?

[smashwilson]

(1) was kind of how I'd been thinking about this before, certainly. Mostly I wanted to run the alternatives past you and see what thoughts you had :smile:

What are the relative advantages of using Redux instead of the context api here?

Some of the more egregious drilled props in our codebase now are action methods - bound functions we pass through N layers of React to get to the actual button or whatever that triggers them. The easiest way I can think to get those to where they need to be is to lift them all up to the root of the component tree.. but then they need to be able to operate on whatever state they had access to at their previous homes in a controller somewhere, so the state would need to be lifted all the way up, too. And then we basically end up with Redux.

Using Redux has the advantage that it's a pretty paved trail. A bunch of experience about state management in React apps is embedded in the library, and engineers who are not us would be responsible for maintenance.

With that said... I don't particularly love it, either. I dislike having state and logic torn apart all across the codebase, and I'm not fond of introducing additional layers of indirection between everything rather than "passing props to components." I find it makes it harder to trace what's actually happening through the app. Caveat: I haven't actually used Redux in anger; I messed around in another project's source that was using... Flux I think?... which was a similar idea, and was thoroughly confused about everything the entire time.

[..] at its heart Redux is really just fancy pubsub

See, I keep thinking I could roll us a less fancy pubsub to suit our needs... that might be the NIH speaking though 😆

I'm leaning toward at least starting with option (1). It should give us the most benefits the fastest, and give us an idea how much mess we'll be able to clean up so we can evaluate the relative merits of boiling the ocean later.