OpenRailAssociation / osrd

An open source web application for railway infrastructure design, capacity analysis, timetabling and simulation
https://osrd.fr
461 stars 44 forks source link

front: technical debt with data transformation #7918

Open anisometropie opened 4 months ago

anisometropie commented 4 months ago

Description and goal

in ManageTrainScheduleV2, some major transformation on pathfinding data is delegated to sub components TypeAndPathV2.

launchPathFinding has much more responsibility than a mere (and small) subcomponent should have

I think that’s a design flaw and a case where props drilling is problematic. it creates major indirection: difficult to follow where to data comes from and what’s done to it.

This really could be a proper case to use a business hook (as proposed by @jacomyal @sim51) to be discussed

TimetableV2

useTrainSchedulesDetails is receiving setTrainSchedulesDetails, which comes from TimetableToolbar props, which comes from a state in TimetableV2.

anisometropie commented 2 months ago

09/09 Meeting report : were present: @anisometropie @SarahBellaha @emersion @kmer2016 @Uriel-Sautron

Observation: there is a lot of different state in different places (instead of having one or fewer sourcers or truth like in redux) and each place has ramifications (setters given to children who do transformation on parents state up 3-4 components hierachy,

Todo : List all the main states used in tsv2. For each main state :

it concerns main business data such as operationalPoints, pathSteps, currently stored in a useState() ``ManageTrainScheduleV2 pathProperties.

If this is data that was initially in redux store, transformed, and store in a different useState(), This could be a case where the data would be better kept in the store and transformed with a selector.

From these observations, think about possible design solutions

Low level transformation should not be exposed in children components. (details of the operations done on the data should only appear in parents) for example in stdcmViewV2

const handleStartNewQuery = () => {
    setSimulationsList([]);
    setSelectedSimulationIndex(-1);
    setRetainedSimulationIndex(-1);
    dispatch(resetStdcmConfig());
  };

Can ManageTrainScheduleV2 setPathProperties be moved to the existing hook usePathProperties ?

anisometropie commented 1 month ago

example: bug https://github.com/OpenRailAssociation/osrd/issues/8935

we have to trace back spaceTimeData up 4 components/hooks to know where it’s created.

SimulationResults => spaceTimeData => ScenarioContent => useScenarioData =>projectedTrains => projectedTrainsById => useLazyLoadTrains => useLazyProjectTrains