Component Patterns: Render prop vs. hook

[thompsongl]

As started in this week's sync: When building a new component where we need to manage internal structure while allowing for custom consumer composition, what pattern should we look to?

I created this gist that has some different options, using the split panel component as the case.

Some initial commentary:

Option 2 is my preference in this case because the correct Outer-Inner relationship is guaranteed, unlike Option 3/4 where the consumer has to understand the intended structure for correct composition.

Option 3 does not make much sense in this case because its only (potential) advantage is saving one import statement:

+ import {useEuiSplitPanel} from '@elastic/eui'`;
- import {EuiSplitPanelOuter, EuiSplitPanelInner} from '@elastic/eui'`;

Now, my opinion would shift quite a bit on this matter if you also wanted to provide state or stateful functions:

const [OuterPanel, InnerPanel, isPanelOpen, openPanel, closePanel] = useEuiSplitPanel();

The implied Outer-Inner relationship problem still exists, but stateful bits are better served coming from a custom hook.

To Chandler's point from the sync, it's debatable whether the render prop pattern is now an anti-pattern in new hook world of React. I generally agree, but in all(?) cases where hook returns UI, we've needed to wrap the hook in an HOC for class components.

[thompsongl]

Also, I found this post while trying to find a different one, but it makes the same point:

One important thing to note. Though the hook is returning a Component, it is really just syntax sugar. The hook is NOT creating a new Component definition each time the hook function executes. This would cause the React reconciler to see everything as a new Component; state would be reset every time.

The last sentence is important when considering a hook that returns a component.

[cchaos]

Thanks for digging into this! I think for me, this seems more like a preference for the consuming side. All the options seem pretty similar (though I wouldn't go the ...props route, that was a bad take on my part). So I think the discussion is mainly around the 2nd and 3rd option.

To me, the main thing I find hard about full hooks (option 3) is that they're less discoverable in the consumer's IDE. I think most of our devs would try to find a component by typing < and seeing what auto-populates. Hook only "components" rely heavily on documentation for discoverability.

But this might be a great candidate to actually shop around for feedback to the different solution teams or a quick survey in our weekly newsletter.

[sebelga]

Got pinged by @cjcenizal on this so I'll bring my 2 cents 😊

I am not a huge fan of hooks to return components. So not option 3.
Option 4 seems a bit over-engineered to me.

So that leaves us with the render props pattern. I wonder why we need to pass down the component like that. What about using context for this? And, if needed, expose a hook to access certain props (e.g. isPanelOpen)?

// context.ts

const context = React.createContext();

export const Provider = () => {
  // Here comes all the different state and handler to pass down
  
  return (
    <context.Provider value={{ /* state and handlers */}}>{children}</context.Provider>
  )
}

export Consumer = context.Consumer;

// Let consumers access state and handlers
export useEuiSplitPanel = React.useContext(context));

// EuiSplitPanel.ts

import { Provider, Consumer } from './context';

const Outer = () => {
  return (
    <Provider>
       <Consumer>
         {() => {
           // If needed to add logic when the context change
           
           return children;
         }}
       </Consumer>
    </context.Provider>
  )
};

const Inner = () => {
  // Has access to all it needs to render through context
  const context = useEuiSplitPanel();
  
  return children;
}


export EuiSplitPanel = { Outer, Inner };

// On the consumer side it is then simply composition, no need for hooks of render props.

// The downside is that the consumer needs to wrap its code in a component
// to be able to later `useEuiSplitPanel()`. The alternative would be to have the <Consumer />
// here but then we are back to the render props

import { EuiSplitPanel } from '@elastic/eui';

export const MyComponent = () => {
  return (
    <EuiSplitPanel.Outer anyProp="bar" canBePassed={true}>
      <EuiSplitPanel.Inner>
        <MyOwnComponent />
      </EuiSplitPanel.Inner>
      
      <EuiSplitPanel.Inner>
        <MyOtherComponent />
      </EuiSplitPanel.Inner>
    </EuiSplitPanel.Outer>
  )
};

[cchaos]

Thanks for the feedback @sebelga! It certainly seems there are several different use cases for something like this. I think the way that you explained is helpful for components that do carry an internal state or other complicated logic based on the consumer's configuration.

The wider question is for more superficial components. The original example, EuiSplitPanel, is simply a pre-defined composition of components. When you mentioned no. 4 being over-engineered, that how I also saw your example for this original use case. 😆

So I'd like to continue this discussion with the pre-defined scope of a component that carries no internal state and no specified logic. In fact, the same could be accomplished simply by using props like:

<EuiSplitPanel panels={[<InnerPanelContent1 />, <InnerPanelContent2 />]} />

Though this doesn't allow easy customization of the individual inner panel's props.

[sebelga]

Oh ok @cchaos . I missread "my opinion would shift quite a bit on this matter if you also wanted to provide state or stateful functions" and thought it was a requirement to have state passed down. 😊

If there is no state, why not go simply with this? I usually find composition nicer than passing props

import { EuiSplitPanel, EuiPanel } from '@elastic/eui';

<EuiSplitPanel>
  <EuiPanel>
    <ConsumerComponent />
  </EuiPanel>
  <EuiPanel>
    <OtherConsumerComponent />
  </EuiPanel>
</EuiSplitPanel>

[cchaos]

No worries, that's why we've got this discussion going because it also sounds easier at the top level than it is. For instance, you newest example would only work if the EuiSplitPanel component then also had to check for valid EuiPanel's as children to be able to pass particular props to them.

This is where composition and flexibility can make things difficult. For this particular case EuiSplitPanel has some very prescribed props for the outer and inner panels in order to make them display correctly. But, we also want to allow further customization of both the inner and outer panels. So it the idea would be:

<OuterPanel {...prescribedOuterProps} {...consumerOuterProps}>
  // Any number of inner panels looking like:
  <InnerPanel[0] {...prescribedInner[0]Props} {...consumerInner[0]Props}>
    {InnerPanel[0]Children}
  </InnerPanel>
</OuterPanel>

[sebelga]

you newest example would only work if the EuiSplitPanel component then also had to check for valid EuiPanel's as children

No it does not need to look at the children. That's what my initial example showed. It's the Panel (inner) that registers itself when it mounts through context. It has a useEffect that says "hey I'm here" (by calling a context handler) to the Outter.

In your above example, the prescribedInner would be hidden away and the consumer props would be added.

// (From my first example)
// I called it Inner but it is the EuiPanel from my second example

const Inner = (consumerProps) => {
  // Has access to all it needs to render through context
  const { registerPanel } = useEuiSplitPanel();
  
  useEffect(() => {
    // Logic to register a Panel to the EuiSplitPanel
    const panelId = registerPanel();
  }, []);
  
  return <div {...prescribedInner} {...consumerProps}>{children}</div>;
}

[cchaos]

Here's a draft PR with an example of using the latest suggestion without the Context support. #4539