Skip to content

Wildcard: simplify Tabs implementation

Administrator requested to merge og/wildcard-simplify-tabs into main

Created by: oleggromov

Why

While doing migration to the new component (https://github.com/sourcegraph/sourcegraph/pull/29920), I came across a bug that I couldn't fix and even understand fully. This all is due to the overly sophisticated implementation of Tabs with a controversial internal state, which pre-rendered and cached panels with respect to lazy and behavior, and some effects, which may have been in a race condition once in a while.

Since the implementation was obviously more complex than it could be, I decided to simplify it.

What's changed

Instead of saving children in internal state, which is passed around as context, we only save two required things:

  1. settings, which are Tabs's props - to be used down the component tree
  2. and index of each rendered panel, which is required to decide shouldPanelRender or not

Everything else wasn't really needed.

Behavior

To re-iterate, Tabs support 3 types of behavior:

  1. lazy={false} - Just render each panel no matter which one is selected
  2. lazy={true} behavior='forceRender' - Only render currently selected panel
  3. lazy={true} behavior='memoize' - Render each panel once and don't un-render once selected index is changed

The 3rd case now has a slightly modified (yet important) behavior. If children of a panel change when it's not selected, it will be un-rendered and re-rendered when it's selected back. Before it wasn't the case and could lead to strange and hard to understand bugs.

Merge request reports