You might notice thatcreateChildren is almost the same asMyTree -- the only difference is thatcreateChildren takes its argument directly instead of as a props object. So with a small change we can removecreateChildren entirely:

const MyTreeNestedItems = (props) => {    const { data } = props;    let children = null;    if (data.children.length) {      children = <MyTree list={data.children} />;    }    return (        <li className="nav-item">            <a className="nav-link" href="#">                <span className={data.icon}></span> {" "}                { data.descp }            </a>            <ul style={{listStyleType:"none"}}>{children}</ul>        </li>    );}

This can be a bit more compact as so:

const MyTreeNestedItems = ({ data }) => {    return (        <li className="nav-item">            <a className="nav-link" href="#">                <span className={data.icon}></span> {" "}                { data.descp }            </a>            <ul style={{listStyleType:"none"}}>                {data.children.length ? <MyTree list={data.children} /> : null}            </ul>        </li>    );};

Currently this code renders invalid HTML -- it renders with a<li> as the top level element but<li> should only be contained in a<ul>,<ol>, or<menu>.

Looking at the code so far,<MyTree> is rendered in two places -- it may be tempting to just add another<ul> around<MyTree list={tree} /> in therender() call. But then we would have two occurrences of a structure like this:<ul><MyTree/></ul>. It'd be easier to just move the<ul> into the<MyTree/>.

With that, the code now looks like this:

const MyTree = ({ list }) => {    return (        <ul style={{listStyleType:"none"}}>            {list.map((child,i)=> <MyTreeNestedItems key={i} data={child} />)}        </ul>    );};const MyTreeNestedItems = ({ data }) => {    return (        <li className="nav-item">            <a className="nav-link" href="#">                <span className={data.icon}></span> {" "}                { data.descp }            </a>            {data.children.length ? <MyTree list={data.children} /> : null}        </li>    );};render(<MyTree list={tree} />, document.querySelector('.js-mytree'));

From here it's a matter of taste. Both of the components are just directly returning a value, so we can use short arrow function syntax now:(...) => (value) instead of(...) => { return (value); }

Also, the components are now simple enough that I would just combine the two.

All in, that leaves us with this final version of the code:

const MyTree = ({ list }) => (    <ul style={{listStyleType:"none"}}>        {list.map(({icon, descp, children}, i) => (            <li className="nav-item" key={i}>                <a className="nav-link" href="#">                    <span className={icon} /> {descp}                </a>                {children.length ? <MyTree list={children} /> : null}            </li>        ))}    </ul>);render(<MyTree list={tree} />, document.querySelector('.js-mytree'));

• \$\begingroup\$Thanks a lot George, that's a great improvement of my code and explanation. Now in a few days I will be focusing on implementing collapse/expand functionality to the component.\$\endgroup\$

  Nestor

  – Nestor

  2019-08-21 11:21:39 +00:00

  CommentedAug 21, 2019 at 11:21

• javascript
• react.js
• jsx