Refactor(pulicodes-state): code review + refactoring

src/components/form/Question.tsx

@@ -8,6 +8,7 @@ import NumberInput from '@/components/form/question/NumberInput'
 import Suggestions from '@/components/form/question/Suggestions'
 import { DEFAULT_FOCUS_ELEMENT_ID } from '@/constants/accessibility'
 import { useRule } from '@/publicodes-state'
+import { NGCQuestionType } from '@/publicodes-state/types'
 
 type Props = {
   question: string
@@ -28,17 +29,10 @@ export default function Question({ question }: Props) {
     activeNotifications,
   } = useRule(question)
 
-  return (
-    <>
-      <div className="mb-4">
-        <Label
-          question={question}
-          label={label}
-          description={description}
-          htmlFor={DEFAULT_FOCUS_ELEMENT_ID}
-        />
-        <Suggestions question={question} />
-        {type === 'number' && (
+  const getInput = (type?: NGCQuestionType) => {
+    switch (type) {
+      case 'number':
+        return (
           <NumberInput
             unit={unit}
             value={numericValue}
@@ -51,8 +45,9 @@ export default function Question({ question }: Props) {
             data-cypress-id={question}
             id={DEFAULT_FOCUS_ELEMENT_ID}
           />
-        )}
-        {type === 'boolean' && (
+        )
+      case 'boolean':
+        return (
           <BooleanInput
             value={value}
             setValue={(value) => setValue(value, question)}
@@ -61,8 +56,9 @@ export default function Question({ question }: Props) {
             label={label || ''}
             id={DEFAULT_FOCUS_ELEMENT_ID}
           />
-        )}
-        {type === 'choices' && (
+        )
+      case 'choices':
+        return (
           <ChoicesInput
             question={question}
             choices={choices}
@@ -73,8 +69,20 @@ export default function Question({ question }: Props) {
             label={label || ''}
             id={DEFAULT_FOCUS_ELEMENT_ID}
           />
-        )}
-        {type === 'mosaic' && <Mosaic question={question} />}
+        )
+      case 'mosaic':
+        return <Mosaic question={question} />
+      default:
+        return null
+    }
+  }
+
+  return (
+    <>
+      <div className="mb-4">
+        <Label question={question} label={label} description={description} />
+        <Suggestions question={question} />
+        {getInput(type)}
       </div>
       {assistance ? (
         <Assistance question={question} assistance={assistance} />

src/hooks/useRules.ts

@@ -35,7 +35,8 @@ export function useRules({ lang, region, isOptim = true }: Props) {
         .then((res) => res.data as unknown),
     {
       keepPreviousData: true,
-      // When we work locally on the model we want the rules to be updated as much as possible (on window focus and every 3 seconds)
+      // When we work locally on the model we want the rules to be updated as
+      // much as possible (on window focus and every 3 seconds)
       refetchOnWindowFocus: process.env.NEXT_PUBLIC_LOCAL_DATA_SERVER
         ? true
         : false,

src/publicodes-state/README.md

@@ -1,15 +1,25 @@
 # Publicodes State
 
-Cette librairie met à disposition des hooks React permettant de gérer le state d'un utilisateur de Nos Gestes Climat et ses simulations associées.
+Cette librairie met à disposition des hooks React permettant de gérer le state
+d'un utilisateur de Nos Gestes Climat et ses simulations associées.
 
-Cette librairie souhaite permettre une façon simple et claire de modulariser le code de Nos Gestes Climat en permettant de séparer toute la logique utilisateur/formulaire/publicodes du reste du front-end. Cette séparation des préoccupations permet au front-end d'évoluer rapidement sans avoir à se soucier des complexités du moteur Publicodes.
+Cette librairie souhaite permettre une façon simple et claire de modulariser le
+code de Nos Gestes Climat en permettant de séparer toute la logique
+utilisateur/formulaire/publicodes du reste du front-end. Cette séparation des
+préoccupations permet au front-end d'évoluer rapidement sans avoir à se soucier
+des complexités du moteur Publicodes.
 
 Ce que cette librairie ne fait pas :
 
-- Chargement des fichiers de règles Publicodes. Un fichier de règle doit être fourni déjà chargé au provider de la librairie.
-- Régionalisation et internationalisation. Le fichier de règle étant fourni par le front-end, la librairie n'a aucune idée de la langue de celui-ci.
+- Chargement des fichiers de règles Publicodes. Un fichier de règle doit être
+fourni déjà chargé au provider de la librairie.
+- Régionalisation et internationalisation. Le fichier de règle étant fourni par
+le front-end, la librairie n'a aucune idée de la langue de celui-ci.
 
-La librairie propose trois providers et leurs hooks associés : un pour la gestion d'un utilisateur, un pour initialiser l'engine et la simulation (basé sur les infos de user) et le dernier pour la gestion d'un formulaire (basé sur la simulation).
+La librairie propose trois providers et leurs hooks associés : un pour la
+gestion d'un utilisateur, un pour initialiser l'engine et la simulation (basé
+sur les infos de user) et le dernier pour la gestion d'un formulaire (basé sur
+la simulation).
 
 ## Providers
 

src/publicodes-state/helpers/getIsMissing.ts

@@ -1,8 +1,8 @@
-import { Situation } from '../types'
+import { RuleName, Situation } from '../types'
 
 type Props = {
-  dottedName: string
-  questionsOfMosaic: string[]
+  dottedName: RuleName
+  questionsOfMosaic: RuleName[]
   situation: Situation
 }
 

src/publicodes-state/helpers/getQuestionsOfMosaic.ts

@@ -1,12 +1,14 @@
+import { RuleName } from '@/publicodes-state/types'
+
 type Props = {
-  dottedName: string
-  everyMosaicChildWhoIsReallyInMosaic: string[]
+  dottedName: RuleName
+  everyMosaicChildWhoIsReallyInMosaic: RuleName[]
 }
 
 export default function getQuestionsOfMosaic({
   dottedName,
   everyMosaicChildWhoIsReallyInMosaic,
-}: Props): string[] {
+}: Props): RuleName[] {
   return (
     everyMosaicChildWhoIsReallyInMosaic.filter((mosaicChild) =>
       mosaicChild.includes(dottedName)

src/publicodes-state/helpers/getType.ts

@@ -1,22 +1,26 @@
-import { NGCEvaluatedNode, NGCRuleNode } from '../types'
+import {
+  NGCEvaluatedNode,
+  NGCQuestionType,
+  NGCRuleNode,
+  RuleName,
+} from '../types'
 
 type Props = {
-  dottedName: string
-  rule: NGCRuleNode | null | any // Model shenanigans: question alimentation . local . consommation is missing "formule"
+  dottedName: RuleName
+  // Model shenanigans: question alimentation . local . consommation is missing "formule"
+  // NOTE(@EmileRolley): I don't get why it's a problem?
+  rule: NGCRuleNode | null | any
   evaluation: NGCEvaluatedNode | null
 }
+
 export default function getType({
   dottedName,
   rule,
   evaluation,
-}: Props):
-  | 'notQuestion'
-  | 'mosaic'
-  | 'choices'
-  | 'boolean'
-  | 'number'
-  | undefined {
-  if (!rule || !evaluation) return
+}: Props): NGCQuestionType | undefined {
+  if (!rule || !evaluation) {
+    return undefined
+  }
 
   if (!rule.rawNode.question) {
     return 'notQuestion'

src/publicodes-state/helpers/safeEvaluateHelper.ts

@@ -1,15 +1,14 @@
-import { Engine, NGCEvaluatedNode } from '../types'
+import { Engine, NGCEvaluatedNode, RuleName } from '../types'
 
 export const safeEvaluateHelper = (
-  rule: string,
+  rule: RuleName,
   engineUsed: Engine
 ): NGCEvaluatedNode | null => {
-  let evaluation = null
   try {
-    evaluation = engineUsed.evaluate(rule)
+    return engineUsed.evaluate(rule)
   } catch (error) {
     // TODO: Sending error to Sentry breaks the app
     console.warn(error)
   }
-  return evaluation
+  return null
 }

src/publicodes-state/helpers/safeGetSituation.ts

@@ -1,18 +1,19 @@
-import { NodeValue, Situation } from '../types'
+import { RuleName, Situation } from '../types'
 
+// FIXME(@EmileRolley): the function should return a Situation
 export const safeGetSituation = ({
   situation,
   everyRules,
 }: {
   situation: Situation
-  everyRules: string[]
+  everyRules: RuleName[]
 }): any =>
   everyRules
-    .filter((rule: string) => situation[rule] || situation[rule] === 0)
+    .filter((rule: RuleName) => situation[rule] || situation[rule] === 0)
     .reduce(
-      (accumulator: Record<string, NodeValue>, currentValue: string) => ({
-        ...accumulator,
-        [currentValue]: situation[currentValue],
+      (situationAcc: Situation, currentRule: RuleName) => ({
+        ...situationAcc,
+        [currentRule]: situation[currentRule],
       }),
       {}
     )

src/publicodes-state/hooks/useActions/index.ts

@@ -1,13 +1,15 @@
 'use client'
 
+import { NodeValue } from '@/publicodes-state/types'
 import { useContext, useMemo } from 'react'
 import { useEngine } from '../..'
 import simulationContext from '../../providers/simulationProvider/context'
 
-type ActionObject = {
+type EvaluatedAction = {
   dottedName: string
-  value: number
+  value: NodeValue
 }
+
 /**
  * A hook to help with the actions display and processing.
  *
@@ -26,10 +28,13 @@ export default function useActions() {
           dottedName: action,
           value: getValue(action),
         }))
-        .sort((a: ActionObject, b: ActionObject) =>
-          a.value > b.value ? -1 : 1
-        )
-        .map((actionObject: ActionObject) => actionObject.dottedName),
+        .sort((a: EvaluatedAction, b: EvaluatedAction) => {
+          if (typeof a.value === 'number' && typeof b.value === 'number') {
+            return a.value - b.value
+          }
+          // NOTE(@EmileRolley): what should be done if the values are not numbers?
+        })
+        .map((actionObject: EvaluatedAction) => actionObject.dottedName),
     [engine, getValue]
   )
 

src/publicodes-state/hooks/useDisposableEngine/index.ts

@@ -2,22 +2,28 @@ import Engine from 'publicodes'
 import { useMemo } from 'react'
 import { safeEvaluateHelper } from '../../helpers/safeEvaluateHelper'
 import { safeGetSituation } from '../../helpers/safeGetSituation'
-import { Situation } from '../../types'
+import { NGCRules, Situation } from '../../types'
 
 type Props = {
-  rules?: any
+  rules?: NGCRules
   situation: Situation
 }
+
 /**
- * A hook that set up a separate engine to use for calculation.
+ * A hook that set up a separate engine, only used to evaluate a specific rule.
+ *
+ * @note There is no impact on the state current application's state.
  *
- * Very ressource intensive. Use with caution
+ * @note It's very ressource intensive, you should use it with caution.
  */
 export default function useDisposableEngine({ rules, situation }: Props) {
   const engine = useMemo(
     () =>
       new Engine(rules).setSituation(
-        safeGetSituation({ situation, everyRules: Object.keys(rules) })
+        safeGetSituation({
+          situation,
+          everyRules: Object.keys(rules ?? {}),
+        })
       ),
     [rules, situation]
   )
@@ -32,11 +38,11 @@ export default function useDisposableEngine({ rules, situation }: Props) {
   const getValue = (dottedName: string) =>
     safeEvaluate(dottedName, engine)?.nodeValue
 
-  const updateSituation = (newSituation: any) => {
+  const updateSituation = (newSituation: Situation) => {
     engine.setSituation(
       safeGetSituation({
         situation: newSituation,
-        everyRules: Object.keys(rules),
+        everyRules: Object.keys(rules ?? {}),
       })
     )
   }

src/publicodes-state/hooks/useEngine/index.ts

@@ -5,7 +5,11 @@ import { NodeValue } from '../../types'
 /**
  * A hook that make available some basic functions on the engine (and the engine itself).
  *
- * It should only be used when it is needed to compare rules between them. If not, useRule should be used
+ * It should only be used when it is needed to compare rules between them.
+ * If not, useRule should be used.
+ *
+ * NOTE(@EmileRolley): could you be more a bit more specific about the usage of
+ * [useEngine] instead of [useRule]?
  */
 export default function useEngine() {
   const { engine, safeEvaluate, safeGetRule, updateSituation } =
@@ -22,7 +26,7 @@ export default function useEngine() {
   const getCategory = (dottedName: string): string => dottedName.split(' . ')[0]
 
   const checkIfValid = (dottedName: string): boolean =>
-    safeGetRule(dottedName) ? true : false
+    safeGetRule(dottedName) !== null
 
   return {
     engine,

src/publicodes-state/hooks/useRule/index.ts

@@ -3,7 +3,7 @@
 import { captureException } from '@sentry/react'
 import { useContext, useMemo } from 'react'
 import simulationContext from '../../providers/simulationProvider/context'
-import { NGCEvaluatedNode, NGCRuleNode } from '../../types'
+import { NGCEvaluatedNode, NGCRuleNode, RuleName } from '../../types'
 import useChoices from './useChoices'
 import useContent from './useContent'
 import useMissing from './useMissing'
@@ -13,11 +13,12 @@ import useType from './useType'
 import useValue from './useValue'
 
 /**
- * A hook to get and set every information about a specific rule
+ * A hook to get and set every information about a specific rule.
  *
- * It should ALWAYS be used to access a rule (unless we need to compare mutliples rules with useEngine)
+ * It should ALWAYS be used to access a rule
+ * (unless we need to compare mutliples rules with useEngine).
  */
-export default function useRule(dottedName: string) {
+export default function useRule(dottedName: RuleName) {
   const {
     engine,
     safeGetRule,

src/publicodes-state/hooks/useRule/useChoices.ts

@@ -4,11 +4,13 @@ import { useMemo } from 'react'
 import { NGCRuleNode } from '../../types'
 
 type Props = {
-  rule: NGCRuleNode | null | any // Model shenanigans: question alimentation . local . consommation is missing "formule"
+  // Model shenanigans: question alimentation . local . consommation is missing "formule"
+  // NOTE(@EmileRolley): I don't get why it's a problem?
+  rule: NGCRuleNode | null | any
   type: string | undefined
 }
 
-export default function useChoices({ rule, type }: Props) {
+export default function useChoices({ rule, type }: Props): string[] {
   const choices = useMemo<string[]>(() => {
     if (type === 'choices') {
       // Model shenanigans: sometimes "une possibilité" is in rawNode, sometimes it is in formule

src/publicodes-state/hooks/useRule/useContent.ts

@@ -1,12 +1,12 @@
 'use client'
 
 import { useMemo } from 'react'
-import { NGCRuleNode, Suggestion } from '../../types'
+import { NGCRuleNode, RuleName, Suggestion } from '../../types'
 
 type Props = {
-  dottedName: string
+  dottedName: RuleName
   rule: NGCRuleNode | null
-  safeGetRule: (rule: string) => NGCRuleNode | null
+  safeGetRule: (rule: RuleName) => NGCRuleNode | null
 }
 
 export default function useContent({ dottedName, rule, safeGetRule }: Props) {