JSX render method in react

Question

The following is a piece of code which works fine, but somehow it looks a bit odd to me to have a function inside a render method and use it in both conditions. Let me know how can I improve this react code as showContainer is only adding a wrapper to the function JSX.

Code -

render () {
 const childJsx = () => (<><h3>Random Child JSX</h3></>)
 return (
 <>
 { showContainer ?
 <Modal>
 {childJsx()}
 </Modal>
 :
 <div className="parent">
 {childJsx()}
 </div>
 } 
 </>
 )
}

Answer 1

Your functionalmethod is fine, just the place You have defined is a bit anti pattern. (But well, if you really want to use that component in JUST in that scope its still fine.)

But let me try to refactor it in a react way:

render () {
 return (
 <>
 {
 showContainer
 ? (
 <Modal>
 <ChildJsx />
 </Modal>
 )
 : (
 <div className="parent">
 <ChildJsx />
 </div>
 )
 } 
 </>
 )
}
function ChildJsx(props){
 return (
 <>
 <h3>Random Child JSX</h3>
 </>
 )
}

React is about reusability. In this case you can use that small component again and again as many as you want.

• Nesh: Thanks for the answer. Can you please explain this You have defined is a bit anti pattern . How can i remove this ?
• Peter: just the place You have defined is a bit anti pattern means that you have defined a fully functional component inside a React.Component. You can have the reason to define it right there, but havent seend that context. If you have no a specific reason to define it in that scope, its better to separate it in a new functional component.(Because of reusability)
• Drew Reese: @Peter React key is completely unnecessary in this case, they are only required (and useful) when rendering arrays. Lists and Keys I also completely disagree with defining functions that return JSX within a component being an anti-pattern, it is very common in fact to do this. Even functional components can be defined and used this way. The only real reason to externalize them is if you want to reuse them in other components, otherwise, this is perfectly valid.
• Peter: @DrewReese As I said, if you have a reason to define a function inside a component, lets do that. But im pretty sure your wrapper is pretty fine as a standalone component. And sooner or later in a bigger project that will be a standalone component.
• Peter: @DrewReese The only real reason to externalize them is if you want to reuse them in other components, otherwise, this is perfectly valid React is about reuasbility. You should HAVE a reason to not externalise the components

Answer 2

You can conditionally select the wrapper instead. Use the ternary to create an internal wrapper component. Not much of a code reduction though, but rather just moving the logic a bit. You can also remove the react Fragments as all the returns are returning single react nodes, which does help reduce the code and IMO improve readability.

render() {
 const childJsx = () => <h3>Random Child JSX</h3>;
 const Wrapper = showContainer
 ? ({ children }) => <Modal>{children}</Modal>
 : ({ children }) => <div className="parent">{children}</div>;
 return <Wrapper>{childJsx()}</Wrapper>;
}

Note: all internally defined render functions or functional components can be defined externally if you desire to use elsewhere in other components.

Answer 3

One could also abstract out the tagname and the value of the class attribute to a variable depending on the value of showContainer. Custom tag name can be used with a variable starting with a capitalized letter. With this approach the code to have the child contents (i.e. {childJsx()}) only needs to be added once.

In the demonstration below, the value for showContainer is set to true when the current time has an even number of seconds.

class App extends React.Component {
 render() {
 const showContainer = (new Date()).getSeconds() % 2
 const childJsx = () => (<><h3>Random Child JSX</h3></>)
 const TagName = showContainer ? 'span' : 'div'
 const className = showContainer ? '' : 'parent'
 return ( 
 <TagName className={className}>
 {childJsx()} 
 </TagName>
 )
 }
}
ReactDOM.render( < App / > , document.getElementById("root"));

.parent {
 color: #f00;
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/react/18.3.1/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/18.3.1/umd/react-dom.production.min.js"></script>
<div id="root"></div>

Answer 4

While Drew and Peter provide some useful techniques when dealing with complex components, this case cries out for the simplest solution possible.

render () {
 if (showContainer) {
 return (
 <Modal>
 <h3>Random Child JSX</h3>
 </Modal>
 );
 } else {
 return (
 <div className="parent">
 <h3>Random Child JSX</h3>
 </div>
 );
 }
}

Why would I start with this where there's so much repeated code? Simplicity is the first step in refactoring. Once you've simplified the problem, you can easily spot where you can make further refactorings once things grow in complexity. As written, this takes only a few seconds to read and understand—something you'll do far more often than writing.