0

I'm trying to delve deeper than the basics into React and am rebuilding a Tree library I had written in plain JS years back. I need to expose an API to users so they can programmatically add/remove nodes, select nodes, etc.

From what I've learned, a ref and context is a good approach. I've built a basic demo following the examples (without ref, for now) but I'm seeing every single tree node re-render when a selection is made, even though no props have changed for all but one.

I've tried a few things like memoizing my tree node component, etc but I feel like I'm failing to understand what's causing the re-render.

I'm using the react dev tools to highlight renders.

Here's a codesandbox demo.

My basic tree node component. I essentially map this for every node I need to show. On click, this calls select() from my context API. The rerender doesn't happen if that select() call is disabled.

const TreeNodeComponent = ({ id, text, children }) => {
  console.log(`rendering ${id}`);
  const { select } = useTreeContext();

  const onClick = useCallback(
    (event) => {
      event.preventDefault();
      event.stopPropagation();

      select([id]);
    },
    [select, id]
  );

  return (
    <div className="tree-node" onClick={onClick}>
      {text}
      {children ? <TreeNodesComponent nodes={children} /> : ""}
    </div>
  );
};

The important part of my context is the provider:

const TreeContextProvider = ({ children, nodes = [], selectedNodes }) => {
  const [allNodes] = useState(nodes);
  const [allSelectedNodes, setSelectedNodes] = useState(selectedNodes || []);

  const api = useMemo(
    () => ({
      selected: () => allSelectedNodes,

      select: (nodes) => {
        setSelectedNodes(Array.from(new Set(allSelectedNodes.concat(nodes))));
      }
    }),
    [allSelectedNodes]
  );

  const value = useMemo(
    () => ({
      nodes: allNodes,
      selectedNodes: allSelectedNodes,
      ...api
    }),
    [allNodes, allSelectedNodes, api]
  );

  return <TreeContext.Provider value={value}>{children}</TreeContext.Provider>;
};

Again, when the call to setSelectedNodes is disabled, the rerenders don't happen. So the entire state update is triggering the render, yet individual props to all but one tree component do not change.

Is there something I can to improve this? Imagine I have 1000 nodes, I can't rerender all of them just to mark one as selected etc.

helion3
  • 34,737
  • 15
  • 57
  • 100
  • Possibly relevant: [Avoid runnning an effect hook when Context get updated](https://stackoverflow.com/a/66544925/13762301) and [Split up Context into state and update to improve performance (reduce renders)?](https://stackoverflow.com/a/66723145/13762301) – pilchard Oct 26 '22 at 16:51
  • The second question above also includes a link to this issue in the React repo which covers various options [Option 1 (Preferred): Split contexts that don't change together](https://github.com/facebook/react/issues/15156#issuecomment-474590693) – pilchard Oct 26 '22 at 16:54
  • Splitting the context might work - so if I'm understanding correctly, I would have a `TreeDataContext` and `TreeApiContext`. The data context would house the node array and tree options object but the api context would house `select()` and other methods I intend to expose? I'll try this. – helion3 Oct 26 '22 at 17:02
  • That's correct, seems like the heart of the problem. As it stands you are sending a new object with newly declared methods to every child on every update, by splitting out the methods, only the children with changed data should be re-rendered within the bounds of React's existing diffing. – pilchard Oct 26 '22 at 17:05
  • Do you have a good resource on how to actually do this? My API context replies on the state data in the other context so isn't that still tying them together? Somehow the api context needs to be able to set state, but I'm not sure how to expose setXXX state methods... at that point I'm just reinventing the api? – helion3 Oct 26 '22 at 17:42
  • See the first duplicate I linked, it has an example using two nested providers declared separately, but composed in a single component. – pilchard Oct 26 '22 at 17:43
  • I've been reworking my code based on that example, and I believe I have done so appropriately but am still seeing every node rerender. I'll keep digging, but I'd appreciate any advice if you have time: https://codesandbox.io/s/epic-meninsky-fzbjqk?file=/src/context.tsx – helion3 Oct 26 '22 at 18:34
  • I've been reworking my code based on that example, and I believe I have done so appropriately but am still seeing every node rerender. I'll keep digging, but I'd appreciate any advice if you have time: https://codesandbox.io/s/epic-meninsky-fzbjqk?file=/src/context.tsx – helion3 Oct 26 '22 at 18:34
  • I’ll write up a sample in a little while, afk at the moment. – pilchard Oct 26 '22 at 18:37
  • I'd appreciate it. I think the problem is my API context relies on the state context, so splitting them up isn't do anything for renders. When you click a node, it calls the api which updates the state context and since the api relies on that state context, and those are both parents of the whole tree, everything updates. I checked the react profiler to explain why things rerender and it confirms that. – helion3 Oct 26 '22 at 22:30
  • Here is a [working sample](https://codesandbox.io/s/recursive-context-splitting-sfh3e6?file=/src/tree.jsx) with limited re-renders. Between the recursion and your desire to access the state through an api in context it is definitely tricky. I think splitting it is still useful, but not on the hard `method vs data` line. Rather I think you'll need to decide what is static API (setters mostly), and what is dynamic (queries against updated state, etc). The example puts `select` in the static API context but puts `isSelected` in with the data where it will have access to updated state. – pilchard Oct 26 '22 at 22:51
  • That's progress, thanks! I can definitely see fewer renders in the react profiler. I've adapted your changes my code and generated 100 nodes and confirmed nothing renders but the clicked node and all `TreeNodesComponent`s, because they have a `useContext`. I'll need to keep experimenting because I want to ensure both APIs are exposed, having two different "apis" isn't ideal. I was wondering is using a store would eliminate the rendering on context change too. I was hoping to keep this react-only for reusability, but I'll do what it needs. – helion3 Oct 26 '22 at 23:21
  • Sounds like a start at least! At some point new data has to be accessed, so it becomes a matter of deciding where to draw that line. – pilchard Oct 26 '22 at 23:27
  • Definitely not ideal to have two API entry points. Made me realize that really what was working in my sample was the line between the last `useTreeDataContext` and the memoized `TreeNodeComponent`. Here's an [unsplit sample](https://codesandbox.io/s/recursive-context-no-splitting-31fl19?file=/src/tree.jsx) that embeds the memoized `api` context object inside the `data` context object passing the whole thing in a single context, but then passing the memoized api function to the children that should be protected from rerendering — seeing the same result as the split version. – pilchard Oct 26 '22 at 23:38
  • Had another brief look at this and using splitting and a`ref` you could do something like [sandbox](https://codesandbox.io/s/context-splitting-with-stateref-recursive-shouldrender-check-cintbo?file=/src/context.jsx). Gives access to the updated state via a `ref` assigned the same `data` object passed to the `dataContext`, rendering is further reduced by supplying a `shouldRender` method, in this case a recursive check if any child nodes have been clicked (you could cache this, but didn't for brevity). The result is that only `TreeNodesComponents` with newly selected children are rendered. – pilchard Oct 27 '22 at 12:54
  • Thanks for the new demo. I had considered doing something like that but figured recursively calculating if a Nodes component should update was going to get expensive and potentially not worth the effort. Someone suggested that instead of a context I use a state and pass the `setState` method down to children, which is the same general idea of splitting the API. It too solved `TreeNode` renders, but can't help the `TreeNodes`. Using some sort of "should render" check is the only thing I can think of. – helion3 Oct 27 '22 at 15:52
  • Simplified it a little using a `proxy` over the `ref` ([codesandbox](https://codesandbox.io/s/context-splitting-with-proxy-state-h95eyx)) which avoids having to rewrite all your accessors. The recursive `shouldRender` could be cached using a look up table tracking parents/children. You could also track `isSelected` as a property on each node, which would require a recursive lookup in the `select` method, but that too could be cached with a lookup table. – pilchard Oct 27 '22 at 15:57
  • Thinking more, I would need some way of knowing that a child's state changed, not just whether it was selected. The node might be selected/deselected, expanded/collapsed, hidden/shown, it might have been added or removed, etc. That's complicating that process quite a bit. I could use some kind of a `dirty` flag, maybe set on all parent nodes when something happens. – helion3 Oct 27 '22 at 16:41
  • Yes, I was uncertain of all the possible methods that may be called against a node, but a `dirty` flag is a good catch all. Setting it on top-level nodes makes sense, React will catch the actual changes and render appropriately. Otherwise you'll need to combine a node-level flag with a cached, recursive `hasDirtyChildren`. Sounds like you might be on the right track. – pilchard Oct 27 '22 at 16:47

0 Answers0