Skip to content

Instantly share code, notes, and snippets.

@kevmo314
Last active July 30, 2024 18:56
Show Gist options
  • Save kevmo314/7bbf5d5868b973b53898d12b977b15b7 to your computer and use it in GitHub Desktop.
Save kevmo314/7bbf5d5868b973b53898d12b977b15b7 to your computer and use it in GitHub Desktop.
Please stop reinventing JSX

Please stop reinventing JSX

It really grinds my gears when I see this pattern, in particular navigation bars and sidebars seem to attract it:

const navItems = [
  {
    title: 'home',
    link: '/home',
  },
  {
    title: 'about',
    link: '/about',
  },
  {
    title: 'contact us',
    link: '/contact-us',
  },
  // ... whatever ...
];

and then later

<ul>
  {navItems.map(({ link, title }) => {
      return (
        <li>
          <Link to={link}>{title}</Link>
        </li>
      );
    })}
</ul>

This is totally unnecessary.

Why is it bad?

The implementer of the nav bar could directly unroll the array:

<ul>
  <li><Link to="/home">Home</Link></li>
  <li><Link to="/about">About</Link></li>
  <li><Link to="/contact-us">Contact Us</Link></li>
</ul>

Why is that better?

For one, it's a lot less code! The unrolled version conveys the same information as the list but instead of adding a level of indirection, it directly codifies what is desired by the developer which makes it much simpler to make changes.

For two, the JSX variant makes it much easier to incorporate element-specific logic. Say, for example, you want a conditional on only one nav item. With an array, you'll have to figure out how to mutate the array or conditionally produce it. JSX already has this baked in!

<ul>
  <li><Link to="/home">Home</Link></li>
  {showAbout && <li><Link to="/about">About</Link></li>}
  <li><Link to="/contact-us">Contact Us</Link></li>
</ul>

There's no need to make your life harder for no reason. The problem is further exacerbated when these conditions are produced in hooks and I've seen some developers go as far as writing a function that produces an array of items that is then converted to JSX.

function getNavItems(someCondition: boolean) {
  return [
    {
      title: 'home',
      link: '/home',
    },
    {
      title: 'about',
      link: '/about',
    },
    {
      title: 'contact us',
      link: '/contact-us',
    },
    // ... whatever ...
  ];
}

// ... later ...

function NavComponent() {
  const someCondition = useSomeHook();
  const navItems = getNavItems(someCondition);

  return <ul>
    {navItems.map(({ link, title }) => {
        return (
          <li>
            <Link to={link}>{title}</Link>
          </li>
        );
      })}
  </ul>;
}

A function that produces nav items... that almost sounds like a React component! So not only have we reinvented JSX, we've completely reinvented the component model except on top of React, so we have to convert from one component model to another.

Why does this happen?

I suspect this antipattern comes from the desire to not hardcode strings. I've seen a lot of developers go through some herculean efforts to avoid hardcoding strings and by extension, an array of objects that we convert to JSX is "more maintainable" than JSX itself.

Except therein lies the problem: React and JSX are declarative languages already.

The desire to not hardcode strings, in my opinion, is a desire to keep imperative logic simpler and move constants to a more declarative world to add semantic information. That makes total sense. What doesn't make sense is moving declarative logic from a declarative world to another declarative world. JSX is already sufficiently descriptive and it's in some sense a giant constant to begin with. Embrace it!

When is not using JSX okay?

There are cases where creating an abstract list does make more sense than using JSX because JSX isn't a universal hammer. For example, in React Router the use of data APIs is essentially moving away from the JSX'ified <Route> pattern.

But this is still in the spirit of the law! The routes here don't represent a list of UI elements, they represent an abstract concept so there is no need to convert it back to React. Whereas in our navigation example, the items directly correspond to UI elements. In that case, React already dictates JSX as the interface. We don't need to create yet another layer of abstraction because we have a perfectly suitable (and also mandated by the framework) one in front of us: JSX.

@L8D
Copy link

L8D commented Jul 30, 2024

I've found myself actually rewriting "unrolled" code into the JSON-based pattern countless times. There are many, many reasons why having the navigation items as a dataset becomes necessary.

  • There can be multiple forms of a navigation menu across your site.
  • There can be a lot of code involved for each navigation item, and so the loop reduces duplication (although you can try to write your own component for reducing this)
  • There can be plenty of metadata and configuration options that apply to other components elsewhere, that need to pull in the "configuration" from the current "route" in which case you need to have a standard way to pass that data around to the other components in a dynamic manner.
    • Say, for example, you want to display the "title" property of the route, and you want that title property to dynamically pull from the same title used for the navigation link.
  • There can be the need to lookup information about a route dynamically, given the name/path of that route. In which case, you need to have the dataset in order to loop through and find the corresponding/canonical definition of that route.

This mostly comes down to the fact that the "route" configuration becomes a high-level global configuration, and you can't have the low-level navigation UI become the source of truth. So instead, the source of truth needs to be a high-level shared value, and the navigation UI needs to pull from the source of truth. This result feels pretty inevitable for most projects that grow and grow, so for seasoned developers this becomes a pattern that we follow because we're really unconcerned about your core argument here.

@hahanein
Copy link

hahanein commented Jul 30, 2024

There can be multiple forms of a navigation menu across your site.

Provide a context then.

There can be a lot of code involved for each navigation item, and so the loop reduces duplication

As you said: Reduce duplication using components.

There can be plenty of metadata and configuration options that apply to other components elsewhere, that need to pull in the "configuration" from the current "route" [...]

Menu entries are not routes not only in that there may be routes that are not referenced by any entry but also in that there may be entries that do not reference any routes. This is a mistake hampering your ability to create highly purpose-build user interfaces.

There can be the need to lookup information about a route dynamically, given the name/path of that route.

Menu entries are not routes. If you need to dynamically look up information about routes, do that. Frameworks like NextJS or Remix do some of that for you but you can implement it yourself. In React Router you could use the "handle" attribute to make this type of information available.

@kevmo314
Copy link
Author

Menu entries are not routes.

To add onto that point...

Say, for example, you want to display the "title" property of the route, and you want that title property to dynamically pull from the same title used for the navigation link.

It strikes me as decidedly-not-seasoned that the page would pull from the navigation link instead of the other way around. The nav link should be pulling from the page, if anything. The data could (and probably should) instead be pulled from the router.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment