Suggestions for technical improvements

[HendrikThePendric]

After the first release of this app, we need to probably take some time to refactor some parts of this app. This GitHub issue can be a place where we collect things we would like to see improved. We can comment here about things that need to be optimised, parts of the code that are hard to work with, or need better test coverage. If you already have a solution, or a global direction of how to address the problem in mind, please include that in the comment.

[HendrikThePendric]

We should probably add a state library like redux so we can have all the application state in one place and are able to subscribe to "slices" of this state in components.

[HendrikThePendric]

We should probably introduce a DataEntryFormInstanceProvider to which we pass a the current react-final-form Form instance. This way the entire application can easily interact with the form and its individual fields using the FormApi. I think that some of the providers we currently have could probably be removed if we had this, and we can also reduce the amount of re-renders this way (I am mainly thinking about using form.registerField and form.subscribe to do this).

[Birkbjo]

If we get structural sharing to work properly I don't think this is necessary. Then react-query would be the only source of truth (except for form-state). And we could use useQuery-targetting specific fields without worrying about re-rendering.

I haven't fully investigated how this works, or if it does. I just think that if it works the way I hope - we don't have to deal with these points at all. Each field could just useQuery and select the part of the cache it wants.

However, one worry could be the amount of subscriptions. If that causes trouble, I agree we could implement some of your points - but the introduction of a DataEntryFormInstanceProvider seems a bit counter-intuitive to me, we should try to have one source of truth, instead of having to sync these manually.

[HendrikThePendric]

I think what you are writing here mainly applies to my suggestion further along. And with regards to that I guess I agree: it would be better if we can work with a single source of truth (or in this case two separate single sources of truth for the form-field-state and the value-sync-state).

But having this DataEntryFormInstanceProvider on its own (i.e. the suggestion above) does seem useful. I think we can rewrite some of the hooks we currently have to not need their own context, but to just use this one instead.

[HendrikThePendric]

We could switch to final-form-calculate for indicators and totals

[HendrikThePendric]

We should look into ways of make the useDataValues() hook return a data object that changes less frequently. Potentially we could use this structural sharing approach to achieve that goal.

Disclaimer: it was @Birkbjo who suggested this to me.

[HendrikThePendric]

From the perspective of the data entry fields (i.e. the inputs), it would make a lot of sense if the entire form-field state was provided by react-final-form. We could take the following approach to achieve this:

• Allow other parts of the app to interact with the form by using the aforementioned DataEntryFormInstanceProvider provider
• Use a RFF Fields data prop to store information needed by the field, but not strictly speaking part of a form field. For example sync status etc.
• Use add mutators to the RFF Form which allow updating these meta fields (aka the data prop).

This way, we can easily update individual fields from the useQuery hooks and we can keep all field data in a single place. I think this would help a lot.

[HendrikThePendric]

We should ensure all queryKeys / queryKeyFactories are implemented and organised in the same way.

[HendrikThePendric]

"Our" data engine returns loading and error, while react-query returns isLoading and isError. In this app we currently have a number of hooks that rename isLoading to loading, but others that don't. So it's a bit of a mess that way. We should decide which naming convention to use and then implement it consistently across the codebase.

[HendrikThePendric]

We tend to use full variable names, such as dataElement, but some of the endpoints used by the app expect abbreviated names, such as de for dataElement. We should use full names everywhere in our code base, and only "translate them to abbreviated names" when we are actually creating a query/request. Currently, some of these abbreviations have infiltrated their way deeper into the codebase, and we should address that.