Conseil clean code React

Les vues en React

Pour ceux qui font du React, on sait que le JSX (le fait de faire du code qui génère du HTML) est très puissant et je mets le TSX dans la même case, c'est uniquement le langage écrit qui change mais il sera lui même compilé en JSX puis interprété pour générer du HTML donc on revient à la même chose.

Le problème de la puissance...

Un grand pouvoir implique de grandes respon... euhh de l'organisation !

Etant donné que l'on peut faire énormément de chose, on serait tenté de faire un peu n'importe quoi tant qu'on y est.

On va mettre des conditions, de la logique etc dans le code de rendu JSX... Ben oui puisqu'il le permet ! Pourquoi pas ?

La confusion

Le problème de mettre de la logique dans la partie JSX de votre React, c'est à dire la partie purement affichage, c'est que vous allez créer beaucoup de confusion au moment de la lecture, pour vos collègues et pour vous même.

Regardez plutôt:

// imports...

export const CalendarContainer = () => {
  // declarations

    return (
      <div id="calendar" className="w-full bg-white p-8">
        {isFetchingPlanning || !planning ? (
          "Loading..."
        ) : (
          <Calendar
            events={planning}
            startAccessor="start"
            endAccessor="end"
            style={{ height: 600 }}
            views={["day", "week"]}
            view={view}
            onView={setView}
            defaultDate={date}
            date={date}
            onNavigate={handleNavigate}
            onSelectEvent={(calendarEvent: CalendarEvent) => {
              switch (calendarEvent.type) {
                case CALENDAR_EVENT_TYPES.JOB:
                  // Worker or Admin
                  if (isWorkerOrAdminSession(session)) {
                    displayJobModal(calendarEvent.id)
                  }
                  // User
                  else if (isOwnerSession(session, calendarEvent.createdBy)) {
                    displayJobModal(calendarEvent.id)
                  }
                  // Not owner nor Worker
                  else {
                    toast.error(session.user ? "Ce créneau est déjà réservé" : "Merci de vous connecter pour faire une réservation")
                  }
                  break
  
                case CALENDAR_EVENT_TYPES.FREE_SLOT:
                  if (session?.user) {
                    displayJobForm(calendarEvent.start, calendarEvent.end)
                  } else {
                    toast.error("Merci de vous connecter pour faire une réservation")
                  }
                  break
              }
            }}
            scrollToTime={new Date(new Date().setHours(7, 0, 0))}
            showMultiDayTimes
            dayLayoutAlgorithm="no-overlap"
          />
        )}
      </div>
  )
}

C'est typiquement le genre de bloc que je retrouve souvent dans les projets et pour moi, il y a un problème.

Le code de rendu JSX ne doit pas contenir de logique.

A la limite une condition d'affichage de bloc en "ternaire" ou "et logique", ou encore une boucle "map" sur les Arrays, mais en aucun cas, faire de la logique complexe.

Le bloc ci dessus a des cas imbriqués avec des parcours en arbre obligeant la personne qui relit le code de réfléchir aux différents cas et à comment sera processé cette logique au moment du rendu.

Ce que j'ai l'habitude de faire

Dans cette situation, je me force à extraire la logique du rendu.

• Soit je peux vraiment la sortir du module et en faire une librairie testable indépendamment, ce qui est le cas idéal.
• Soit je fais juste une fonction interne au composant en dehors du rendu mais avec la possibilité de simplifier la lecture du rendu.

Je le répète, un rendu sert à afficher, rien d'autre.

Dans ce cas précis, on cherche à faire l'affichage d'une modal ou d'un formulaire en fonction du type d'event sur un calendrier.

• Si l'évent est un JOB on veut afficher une modal de détails si l'utilisateur est le Worker affecté à ce job ou l'Admin, ou s'il est la personne ayant créé le job, dans le cas contraire on affiche un toaster d'erreur.
• Si l'évent est un FREE_SLOT on veut afficher un formulaire qui permet de prendre une réservation, peu importe quel est le rôle de mon utilisateur.

Ca commence à faire beaucoup de logique et de use cases non ?

Je me dis, que dans ce cas, il est préférable de créer une fonction pour gérer le click sur un JOB et une autre fonction pour gérer le click sur un FREE_SLOT.

L'idée est de faire sortir toute la logique de parcours et de cas d'utilisation de mon rendu qui ne doit servir qu'à l'affichage, comme ceci:

export const CalendarContainer = () => {
  // déclarations...
  
  const tryToDisplayJobModal = (calendarEvent: CalendarEvent) => {
   // Worker or Admin
    if (isWorkerOrAdminSession(session)) {
      displayJobModal(calendarEvent.id)
      return
    }
    // User
    else if (isOwnerSession(session, calendarEvent.createdBy)) {
      displayJobModal(calendarEvent.id)
      return
    }
    // Not owner nor Worker
    toast.error("Ce créneau est déjà réservé")
  }

  const tryToDisplayJobForm = (calendarEvent: CalendarEvent) => {
    if (session?.user) {
      displayJobForm(calendarEvent.start, calendarEvent.end)
    } else {
      toast.error("Merci de vous connecter pour faire une réservation")
    }
  }

  return (
    <div id="calendar" className="w-full bg-white p-8">
      {isFetchingPlanning || !planning ? (
        "Loading..."
      ) : (
        <Calendar
          events={planning}
          startAccessor="start"
          endAccessor="end"
          style={{ height: 600 }}
          views={["day", "week"]}
          view={view}
          onView={setView}
          defaultDate={date}
          date={date}
          onNavigate={handleNavigate}
          onSelectEvent={(calendarEvent: CalendarEvent) => {
            switch (calendarEvent.type) {
              case CALENDAR_EVENT_TYPES.JOB:
                tryToDisplayJobModal(calendarEvent)
                break

              case CALENDAR_EVENT_TYPES.FREE_SLOT:
                tryToDisplayJobForm(calendarEvent)
                break
            }
          }}
          scrollToTime={new Date(new Date().setHours(7, 0, 0))}
          showMultiDayTimes
          dayLayoutAlgorithm="no-overlap"
        />
      )}
    </div>
  )
}

Comme on peut le voir dans cet exemple, il n'y a plus de logique poussée et de tests dans notre rendu JSX. On se contente d'appeler une fonction d'affichage d'un composant et c'est cette fonction qui se chargera de la logique d'affichage. Le but est de réduire la complexité de lecture de votre composant.

Notez que c'est une des solutions et pas la solution ultime pour répondre à cette problématique. On pourrait tout aussi bien créer une fonction qui génère une clé du cas dans lequel on se trouve: IS_ADMIN, IS_WORKER, IS_OWNER, etc... et faire juste un switch case ensuite. Bref, il y a pléthore de possibilités, le but était juste de simplifier le bloc initial.

Pour reprendre les principes du Clean Code, le code est plus souvent lu qu'écrit, c'est pourquoi, il faut attacher une attention particulière à améliorer l'expérience de lecture, soit faciliter compréhension du code pour vos lecteurs.

Bon code à vous 😉