DEV Community

Rafael Cachoeira
Rafael Cachoeira

Posted on • Updated on

React Refactoring - Composition: When don't use it!

#react #refactoring #webdev #beginners

Intro

I refact a dropdown composition component. Changing it to object.

We had a dropdown in the following structure:

<Dropdown>
   <Dropdown.Toggle color='primary' />
   <Dropdown.Menu>            
      <Dropdown.Item onClick={() => {}}>
         MyLabel
      </Dropdown.Item>
      <Dropdown.Item onClick={() => {}}>
         AnotherLabel
      </Dropdown.Item>
   </Dropdown.Menu>
</Dropdown>
Enter fullscreen mode Exit fullscreen mode

Problem

When we need to hide some elements, we need to apply conditional rendering and with that, we have a mixture of javascript with jsx syntax that makes the code difficult to read and difficult to maintain. For example:

<Page>
   <Subheader helpText="Choose a Publication action">
      <Dropdown>
         <Dropdown.Toggle color='primary' />
         <Dropdown.Menu>
            {allowEdit ? 
               (                  
                  <Dropdown.Item onClick={handleTransfer}>
                     Transfer
                  </Dropdown.Item>
                  <Dropdown.Item onClick={handleAbandon}>
                     Abandon
                  </Dropdown.Item>
               ) : (                        
                     {hasPermission('Pages.Publications.Publish') ? (                     
                        <Dropdown.Item onClick={handlePublish}>
                           Publish
                        </Dropdown.Item> 
                     ) : (
                        <Dropdown.Item onClick={handleTransfer}>
                           Transfer
                        </Dropdown.Item>
                     )}
                  )
               )}
         </Dropdown.Menu>
      </Dropdown>            
   </Subheader>
   <Content>
   ...
   </Content>
<Page>
Enter fullscreen mode Exit fullscreen mode

Realize that there is a ternary inside the other, and also, there is a code duplication for the 'Transfer' item. This is terrible to read.

If allowEdit is TRUE, it should render the 'Transfer' and 'Abandon' items:

transfer and abandon items

If allowEdit is FALSE and user has permission to Publish, it should render only 'Publish' item.
Or 'Transfer' for fallback.

just publish item

transfer as fallback item

Proposal

Create a new component (DropdownButton) to support options that receive list items instead of children. Like this:

<DropdownButton color='primary' options={[
   { label: '', onClick: () => {}, visible: true }
   { label: '', onClick: () => {}, visible: true }
  ]}
/>
Enter fullscreen mode Exit fullscreen mode

The visible property should control when showing the dropdown item.

Benefits

  • The code is easier to read
  • Less code repetition
  • We can import the object from elsewhere

And applying our conditional rendering...

<Page>
   <Subheader helpText="Choose a Publication action">   
   <DropdownButton
      color='primary'
      options={[
         {
            label: 'Transfer',
            onClick: handleTransfer,
            visible: allowEdit || !hasPermission('Pages.Publications.Publish')
         },
         {
            label: 'Abandon',
            onClick: handleAbandon,
            visible: allowEdit
         },
         {
            label: 'Publish',
            onClick: handlePublish,
            visible: !allowEdit && hasPermission('Pages.Publications.Publish')
         },
      ]}
   />
   </Subheader>
   <Content>
   ...
   </Content>
<Page>
Enter fullscreen mode Exit fullscreen mode

Conclusion

We are used to writing more declarative code, always thinking about generic code, but sometimes it makes our code difficult to read and maintain.
Props as object items are widely used and we can use them when we want to better control our components. In this refactoring, we've assigned responsibility for the conditional rendering requirements to the visible prop.

Top comments (4)

Collapse
 
brense profile image
Rense Bakker •

Composition is almost always better than not using composition. Its kind of a bad practice in React to create components that take in huge config objects through props, because it indicates that component has way too many responsibilities. The usage might look clean, but the implementation won't. If you need to conditionally render something, its trivial to add a wrapping component:

function Show({ condition, children }:props){
  return condition ? children : null
}

//usage
<Show condition={true}>
  <YourComponent />
</Show>
Enter fullscreen mode Exit fullscreen mode

Just keep in mind that YourComponent is mounted regardless of the outcome of the condition, but it won't appear in the DOM if the condition is false.

Collapse
 
raafacachoeira profile image
Rafael Cachoeira •

Yes, I agree that very complex props can take a lot of responsibility, but in this case it's a simple dropdown with label + handle + visible. I prefer :D
Thanks for the comment.

Collapse
 
brense profile image
Rense Bakker •

Yes but at some point in the future you want to add some other functionality, so you adjust the config and add even more complexity to your single master component. With composition, if you want to add some special menu item, you don't have to change any code, you just extend the existing menu item by wrapping it in your new special functionality menu item component. Having to change the responsibilities of an existing component, when a client asks for new functionality, is a strong indicator that your code is too rigid.

Composition in React is a very valuable skill to learn.

Collapse
 
aarone4 profile image
Aaron Reese •

Nice

Read next

slobodan4nista profile image

The Compounded Interest of Good Software: An example of Carousel Components

Slobi -

paulund profile image

How to debug axios requests

Paulund -

thisisjofrank profile image

An intro to TSConfig for JavaScript Developers

Jo Franchetti -

leemeganj profile image

Jest adoption guide: Overview, examples, and alternatives

Megan Lee -