KaotoIO / kaoto-ui

Frontend for the Kaoto project to provide an easy-to-use integration framework based on Apache Camel.
https://www.kaoto.io
Apache License 2.0
89 stars 44 forks source link

Refactor: Clearer separation of concerns for visualization #1183

Open kahboom opened 1 year ago

kahboom commented 1 year ago

Please describe the task that needs to be done

We recently took some steps to extract functions into a service where they could more easily be tested, however, there is still a lot of room for improvement. The components, in particular, should be refactored to be part of the presentation layer without any in-app state (where possible), and we can move the business logic into their own components. This will also make it much easier to bring back Storybook (#1100).

Similarly, the visualizationService is becoming a really large file and contains a lot of business logic, where many of the functions are not specific to the visualization. For example, findStepIdxWithUUID has nothing to do with the visualization, but it still part of the service. We should break really long file into two different ones with relevant concerns.

subtasks

igarashitm commented 1 year ago

I'm planning to introduce stepService.tsx and move internal step representation (IStepProps) handling from visualizationService.tsx to stepService.tsx. Naming aside :grin: Letting visualizationService.tsx focus on visualized steps (nodes/edges). We could even wrap those service functions into a class - so those in visualizationService.tsx will be in VisualizationService class, and StepService class as well, so that the functions for same concern are bundled with a name. WDYT?

kahboom commented 1 year ago

I think that sounds really good! The only part I'm not sure about is introducing classes into the code base, but it's because even though I think and organize the code in models I try to keep it very functional programming. To bundle together you can group them into an object too, like:

const ExampleService = {
  exampleMethod: function(value) {
    // something here
  },

  anotherMethod: function(value) {
    // ...
  }
};

export default ExampleService;

And use it like:

import ExampleService from "@kaoto/services";
ExampleService.exampleMethod('hello');

The only reason I had them as individual functions not wrapped is because I personally prefer a shorter way to access the method like we do now:

import {
  exampleMethod
} from '@kaoto/services';
exampleMethod('hello');

But of course the problem is that then the imports can get very large, and you don't have them grouped or know which service it comes from. So, in that sense I think your approach makes more sense, because you are importing a service from @kaoto/services and it would be nice to see which one it is.

kahboom commented 1 year ago

@igarashitm - Should this be closed now from https://github.com/KaotoIO/kaoto-ui/pull/1228?

igarashitm commented 1 year ago

I'd like to keep it open until I finish the rest of the components, WDYT?

kahboom commented 1 year ago

Ah yes, sorry, I got confused!

lhein commented 1 year ago

lets move it to sprint 16 then or even later