Appliquer les données de l'open data #3211
Conversation
|
|
||
| export type IndicateurImportSource = { | ||
| id: string; | ||
| libelle: string; | ||
| type?: SourceType; |
There was a problem hiding this comment.
Pourquoi optionnel ?
Dans quel cas une source peut ne pas avoir de type ?
There was a problem hiding this comment.
Parce que ce champ n'existe pas encore (pour l'instant on a que des sources de type "resultat").
Je te propose de changer ça quand le nouveau modèle sera disponible et que je ferai les changements dans le front.
| /** données à appliquer */ | ||
| openData: TIndicateurValeurEtCommentaires[] | ||
| ) => { | ||
| if (!openData?.length) { |
There was a problem hiding this comment.
Au vu du type array non null et non undefined de openData, pourquoi pas simplement openData.length (sans la protection ?) ?
| setCurrentSource, | ||
| }: { | ||
| definition: TIndicateurDefinition; | ||
| sources?: IndicateurImportSource[] | null; |
There was a problem hiding this comment.
Je vois beaucoup de props et de variables gérées en type undefined ou null qui ajoutent ensuite pas mal de petites syntaxes polluantes dans le code pour vérifier cet état non défini.
Autant parfois c'est nécessaire, autant ici, et dès qu'on peut, il me semble plus simple de gérer un array vide s'il n'y a pas de sources plutôt que de vérifier le null et undefined à chaque fois.
Quand bien même ça tire le fil de cette logique sur useIndicateurImportSources et useImportSources.
| useIndexedSources(sources); | ||
|
|
||
| // source sélectionnée | ||
| const s = sources?.find(s => s.id === currentSource); |
There was a problem hiding this comment.
Pas possible d'utiliser un "return early" dans le cas ou s n'est pas défini, i.e que il n'y a pas de source sélectionnée ?
Ça éviterait pas mal de gestion du undefined ou null dans la suite du code, et permettrait de simplifier la lecture.
No description provided.