Internal: Clean up how some React hooks are being used

[dmfalke]

There are some hooks with the same name (useStudyMetada), and some hooks that really should not be hooks (useSession).

[bobular]

Bit confused by both assertions above. Especially "hooks that should be hooks"? Discuss at next Weds meeting?

[dmfalke]

That was a typo (a common class of typo for me -- forgetting the negation :slightly_smiling_face:). I updated the description.

[dmfalke]

Some of our hooks fetch data from a backend service. We would typically only use these once in an application (or maybe a page of the application). One example is useSession. It can be argued that these should not be hooks, since we really don't want them used multiple times in the same application.

I don't have a clear picture of what things should look like. This issue is just meant to serve as a reminder to clean things up.

[dmfalke]

I have some more ideas on how to address this.

The following hooks should be replaced with "container components". By this, I mean that the logic in the custom hook should instead be encapsulated in a component.

• analysis.ts
• entityCounts.ts
• study.ts

These components would then pass that part of the application state as props to child components. I would also like to avoid using React.Context for accessing state, for the time being. Doing so adds a layer of indirection, can potentially cause multiple re-renders when state is updated, and introduces a class of runtime errors that cannot be caught at compilation time.

I'd like to work on this asap, so that we don't incur too much more technical debt in this area.

[dmfalke]

Note, this will also help with managing some aspects of performance, since it will be easier to track how state is being passed around in the application.

[dmfalke]

I'm going to wait on this.

[bobular]

It may also help with making duplicate requests to the count endpoint.

[bobular]

There are currently 18 count requests for GEMS1 - now that birds eye is merged.

There are 6 entities, so for each entity there are

• 1 x count requests with zero filters
• 2 x count requests with filters (filters look identical in browser dev tools)

I wonder if the memoization in the usePromise in core/hooks/entityCounts.ts is not working as intended for the filters dependency? Does it need JSONification?

We could reduce it to 12 requests if this is addressed, maybe?

[dmfalke]

Thanks for the details, @bobular. One of the changes I have in mind includes smarter resource caching. This will address the repeated counts calls, and also basic navigation between different variables, visualizations, etc.

The basic premise is that many of these requests can be cached in memory, using studyId + filters as a key. If either of those change, we would clear the cache. There might be cases where we need even more info to generate a cache key, such as a viz config, especially if we want to cache visualization calls.