Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Star 7 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save markerikson/faac6ae4aca7b82a058e13216a7888ec to your computer and use it in GitHub Desktop.
Save markerikson/faac6ae4aca7b82a058e13216a7888ec to your computer and use it in GitHub Desktop.
Reactiflux chat log: React-Redux, "stale props", and subscriptions

[9:13 PM] harry : @acemarke are people still going to need to add Connect components to their jsx to use a redux hook? i couldnt tell where that ended up
[9:13 PM] harry : didnt 100% grok the whole thing
[9:13 PM] acemarke : awright, lemme recap the issue
[9:13 PM] acemarke : :)
[9:13 PM] harry : sweet
[9:13 PM] acemarke : from the top
[9:13 PM] harry : i kinda got the gist of the zombie child thing too. not sure how a child actually subscribes before a parent, though
[9:14 PM] acemarke : (drat... I can already tell this is gonna be one of those chats I have to export to a gist because I'm about to write a lot)
[9:15 PM] acemarke : up through v4, there was a potential bug due to the timing of store subscriptions
[9:15 PM] acemarke : wrapper components subscribe in componentDidMount, which fires bottom-up
[9:15 PM] acemarke : so if I render <A><B><C> all at once, C's cDM fires first, then B, then A
[9:16 PM] acemarke : that meant that if I rendered a tree of connected components all at once, the bottom component would subscribe before the parent, etc
[9:16 PM] acemarke : now imagine that a Connect(List) is pulling out an array of child item IDs and rendering <ConnectedListItem id={123} />
[9:17 PM] acemarke : and the list item is returning, say, {name : state.items[ownProps.id].name}
[9:17 PM] acemarke : now dispatch {type : "DELETE_ITEM", id : 123}
[9:17 PM] acemarke : reducer runs, state updates, subscribers are called
[9:18 PM] acemarke : ideally, the parent's callback runs first, it re-renders without that ID, and the child is erased from existence
[9:19 PM] acemarke : but, if the child subscribed first, its mapState runs, it tries to read that no-longer-existing value from the store, and BOOM. Cannot read property "name" of undefined
[9:19 PM] acemarke : because the props were "stale" with regards to the store state
[9:20 PM] acemarke : ie, the component really should have had the updated props from its parent (or in this case, no longer even be mounted), but the subscription ordering didn't match the render tree ordering
[9:20 PM] acemarke : with me so far?
[9:21 PM] harry : yep yep!
[9:21 PM] acemarke : okay, so that's the original issue
[9:22 PM] acemarke : v5 fixed this by introducing a custom Subscription class. Each connected component overwrites context.subscription with its own instance, forming a tree of nested subscriptions
[9:22 PM] acemarke : so that only the top few connected components are actually subscribed to the store
[9:22 PM] acemarke : nested components are actually subscribed to their nearest connected ancestor
[9:22 PM] harry : i know that's a problem because people don't do it, but couldnt that theoretically be solved by them guarding against potential undefineds?
[9:22 PM] acemarke : hypothetically, yes
[9:22 PM] harry : still listening tho, continue
[9:23 PM] acemarke : the subscription hierarchy causes a series of cascading updates - each connected component checks to see if it needs to re-render itself, and if so, waits to trigger nested subscribers until its componentDidUpdate. if no update is needed, the update is cascaded immediately
[9:24 PM] acemarke : that way, a child component is guaranteed to not read from the store until it has actually received updated props
[9:24 PM] acemarke : in v6, we got to throw all that code away, because the store state was propagated via new context, and the mapState reads were all done in the wrapper components' render() methods
[9:25 PM] acemarke : so React naturally enforced the top-down updates for us automatically, because that's how it renders
[9:25 PM] acemarke : buuuut, because of the perf issues with context, we had to go back to custom Subscription behavior for v7
[9:25 PM] acemarke : same nested subscriptions, just different internal implementation for the connect wrapper components (func comp + hooks, vs classes)
[9:26 PM] acemarke : and, in order to get that nesting, the connect function components actually have to render the context provider, and create a new context value object with their own Subscription instance
[9:26 PM] acemarke :

      // Determine what {store, subscription} value should be put into nested context, if necessary,  
      // and memoize that value to avoid unnecessary context updates.  
      const overriddenContextValue = useMemo(() => {  
        if (didStoreComeFromProps) {  
          // This component is directly subscribed to a store from props.  
          // We don't want descendants reading from this store - pass down whatever  
          // the existing context value is from the nearest connected ancestor.  
          return contextValue  
        }  
  
        // Otherwise, put this component's subscription instance into context, so that  
        // connected descendants won't update until after this component is done  
        return {  
          ...contextValue,  
          subscription  
        }  
      }, [didStoreComeFromProps, contextValue, subscription])  

[9:27 PM] acemarke : and now we get to the Redux hooks issue
[9:27 PM] acemarke : connect can override that context value, because it's rendering the wrapped child component, and it can put a <ReactReduxContext.Provider> around it
[9:27 PM] acemarke : a hook can't
[9:28 PM] acemarke : because a hook can't render anything
[9:28 PM] acemarke : and definitely not a context provider
[9:28 PM] acemarke : so, let's imagine a React-Redux app with a <Provider><App /></Provider>, and no use of connect() anywhere
[9:28 PM] acemarke : just useSelect()
[9:28 PM] acemarke : every hook is subscribed to the Subscription instance inside of <Provider>
[9:29 PM] acemarke : and now you're basically back to the situation as it was in v4
[9:29 PM] acemarke : where subscriptions could be executing with stale props being used to read values
[9:29 PM] harry : mmm
[9:30 PM] harry : so the Connects would be manually adding those nested subscriptions?
[9:30 PM] acemarke : basically, yeah
[9:30 PM] acemarke : now, note that this isn't a problem if you're just reading values straight from the store, without basing it on props at all
[9:31 PM] harry : right right
[9:32 PM] acemarke : so as MrWolfZ pointed out in that thread, there's a few possible ways this can play out:
[9:32 PM] acemarke : - you've got the right data already, so the extraction is correct
[9:33 PM] acemarke : - you've got the wrong inputs, and it winds up throwing an error
[9:34 PM] acemarke : - you've got the wrong inputs, there's no error, but by the time the component gets around to rendering it does have new wrapper props, so we force a re-read from the store in the middle of rendering and now it's correct
[9:34 PM] acemarke : so his suggestion was basically "just let it go"
[9:35 PM] harry : so just let the risk happen of people experiencing errors?
[9:35 PM] acemarke : roughly, yeah
[9:36 PM] harry : i mean, instead of adding a Connect component, which may be asking a bit much of regular developers, they could just add guards to their selector, if i understand correctly. if i get the zombie child problem, all of my children would be zombies wouldnt they? this would be a consistent thing, right? not some kind of race condition?
[9:37 PM] harry : by asking a bit much, i just mean that they may not have foresight into the problem
[9:37 PM] acemarke : it's... sorta race-condition-ish
[9:38 PM] acemarke : like, if I load my list component first, then the children later, there's no issue
[9:38 PM] acemarke : if I load the list and a bunch of initial children first, then there's an issue
[9:38 PM] harry : wait, will the same list and children load before and after each other?
[9:38 PM] acemarke : it's all about the order of the calls to store.subscribe()
[9:39 PM] harry : right, so is that part race condition-y? one time i load my app the list subscribes first, then the children, and vice versa the next time?
[9:39 PM] acemarke : it's deterministic, in that if you tell me exactly what data you have when the components render the first time, I can tell you if there's an issue
[9:40 PM] acemarke : it's race-y, in that you may not have that data there every time
[9:40 PM] harry : fun. i also saw something about proxies? i wasnt really familiar with them, but read up.
[9:41 PM] acemarke : again, specifically: it's when a tree of connected components all renders for the first time together, and since cDM fires bottom-up, the child subscribes before its parent does
[9:42 PM] acemarke : whether that happens is entirely dependent on the shape of your app and what data you have
[9:42 PM] harry : ohh wait i was a little confused how in that situation the list could subscribe before children, based on that, but if the list mounts/subscribes before it has any data to show children
[9:42 PM] acemarke : yeah, lemme rephrase it this way
[9:43 PM] acemarke : - list of 10 items already exists in the store, turn on "show list", list immediately renders and renders children: has the zombie child issue
[9:43 PM] acemarke : - turn on "show list", list renders, data fetched, children render later: does not have the issue
[9:43 PM] harry : yep yep, thats what i was getting at
[9:45 PM] acemarke : so Tim's suggestion was to have the hooks return some kind of an intermediate component you could include in your render output that would provide a nested subscription
[9:46 PM] harry : yeah. they'd just have to know to use it. i guess it's also not something you'd have to use every time you used the hooks, just in that potential zombie child case
[9:46 PM] acemarke : all the "proxy" stuff is basically more about whether or not you should still be writing your own mapState / selector functions, or have some kind of "magic" usage tracking instead
[9:47 PM] harry : wat
[9:47 PM] harry : so doing some kind of things between gets
[9:47 PM] acemarke : ES6 Proxies are kind of like Ruby's missing_method
[9:48 PM] acemarke : (never used Ruby, but I've heard of that API)
[9:48 PM] harry : not familiar with that, but i did read up on proxies last night
[9:48 PM] acemarke : basically, you can implement functions that will get called any time a piece of code tries to access a field
[9:48 PM] acemarke : and your callback will be given the field name
[9:48 PM] acemarke : and then you can do whatever you want
[9:48 PM] acemarke : return the actual underlying value in the wrapped object
[9:48 PM] acemarke : return a constant
[9:48 PM] acemarke : not return anything
[9:49 PM] acemarke : record some data
[9:49 PM] acemarke : whatever
[9:49 PM] acemarke : that's how Immer works - it wraps your original state in a Proxy, tracks all the mutations you tried to make, then smartly applies the immutable equivalents to create the return value
[9:50 PM] harry : ahhh! neat
[9:51 PM] acemarke : so what theKashey and dai-shi have done is create various libs that uses proxies to track changes to objects, etc, then apply those to wrapping the Redux state so that you can just use state.some.field in your component, and it figures out "oh, the user just wants to update this component whenever state.some.field changes"
[9:51 PM] acemarke : rather than writing const mapState = (state) => ({field : state.some.field})
[9:53 PM] harry : any idea what the api for that looks like?
[9:53 PM] acemarke : like, internal implementation, or usage?
[9:53 PM] harry : sounds interesting
[9:53 PM] harry : usage
[9:53 PM] acemarke :

const state = useReactiveRedux();  
  
return <div>Name: {state.field.name}</div>  

[9:54 PM] acemarke : (roughly)
[9:54 PM] harry : and that's your whole state tree?
[9:54 PM] acemarke : wrapped in a proxy, yes
[9:55 PM] harry : i like that actually
[9:55 PM] acemarke : biggest issue is that proxies are an ES6 only feature, that mostly can't be polyfilled
[9:55 PM] acemarke : some kind of polyfill does apparently exist, but it's limited
[9:56 PM] harry : oh yeah i saw that. so would this thing even live in react-redux land or be its own thing?
[9:56 PM] acemarke : well, the actual reactive-hooks-redux or whatever it is is a separate unofficial lib
[9:57 PM] acemarke : faceyspacey was arguing that we should be hopping on that train right now
[9:57 PM] harry : its kind of crazy how many people use react-redux
[9:57 PM] acemarke : yup, exactly
[10:13 PM] acemarke : I should probably clarify that you can still end up with "stale props" issues even without that initial subscription scenario
[10:13 PM] acemarke : and that would be really any time the parent components would be passing down new props as a result of the store update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment