Architecture proposal: Refactor of `pipeline_editor_app` component
Description of the problem
pipeline_editor_app
is big already and it will keep growing as we add features. This component is currently in charge of:
- Fetching the data for the editor, visualization and linter
- Commit logic to save changes
- Handle success and error banners for the section and the editor
- Show multiple loading icons for each tabs and its own content
This make the component big, hard to reason with and test. It will also create a lot more merge conflicts in the section since everything will be pushed into this component as the section grows.
Goals
This refactor should leave us with smaller components that are easy to test and have a better separation of concerns.
Architecture proposal
We should start by breaking the component in 3:
- The topmost logic that fetches the data should be its own component and we can keep the name
pipeline_editor_app
- Under this component, we would have a new component
pipeline_editor_home
(I am up for suggestion on this name) - We would break out the
commit
logic and make it its own component,pipeline_editor_commit
Pipeline editor app
This component would fetch the graphql queries in pipeline_editor_app
(blob_content
and ci_config.graphql
) and render errors that are related to the network. If there is an error fetching the data, we should ensure that this is a state wide error and that each individual tab does not need to handle this state.
Question Should we disable the tabs while it loads and only enable them if there is no error? Or perhaps we could show both notification on top of another, which would convey the right message. For example, the visualization state would be: (failed to load => viz data is empty).
The data this component fetches could be passed down as prop, but the middle component pipeline_editor_home
would only pass it down as well seems sub-optimal. The 2 options here would be:
- Create local graphQL queries to access the cache from the children components
- Use
provide/inject
to pass down the data, and we can have the childrensubscribe
when they need to.
Question Does provide inject has the same problem where we would pass the data directly through the DOM? And follow-up, does graphQL local queries sound like a good idea for child component to fetch the data they need?
Pipeline editor home
This would serve as the middle(wo)man between the fetched data and the components we want instantiated, a wrapper in a sense. So the idea would be that this component simply adds each of the main component in a tab (editor, viz, and lint).
It would be agnostics to errors and any business logic. We will be forced to have it contain some room for an alert because of the commit form where the form is at the bottom, but we would want the error to render at the top. So its child will emit an event if it needs to display an error at the top (generic pattern we can reuse elsewhere)
So in essence, it should simply render the components. If there are additional logic to handle, let's create them as their own new "parent" component. For example, pipeline_editor_commit
will take all the logic to commit and instantiate the form (see below)
Pipeline editor commit
pipeline_editor_commit
will take all the logic to commit and instantiate the form. It will receive the editor content (through either inject or graphQL query) and when you click on submit, it take care of the business logic and the error message that can happen. If there is a commit error, we emit an event that will be caught by the wrapper and display the appropriate error. The child will pass all the data for the error so that the parent stays agnostics as to what kind of error it can receives.
Data story
Because a picture is worth a thousand words: