Improve useModuleInfo(path) hook

stripes-core is old and has lots of cruft, which is to say, feel free to ask questions and don’t assume what’s there is a best practice so much as something we haven’t updated in ages. Your words were fine, your questions were good; I’m the one who climbed up on a soapbox. Slack#stripes is a good place to ask questions since there are lots of people on that channel, both to answer questions and to learn from them.

thanks for the tips, I'll try to dig into Option 1. I see that we make a request after login to /modules?full=true, I will try to extract data from one of the places you mentioned. I haven't worked with stripes-core at all, so this is my chance to figure it out. I apologize if my words sounded somehow harsh, or the question was asked incorrectly, the goal was to get feedback on the options I proposed, and to find out if anything needs to be done in general, thanks for understanding

, Sorry for my knee-jerk reaction. You asked some specific, thoughtful, well-intended questions about how to improve the architecture of stripes-core, and I mostly ignored them while bashing out a lecture about architecture. How ironic and embarrassing.

I think both option 1 and option 2 are good ideas, though I don’t know if useModules gives you what you need. It only provides UI module data gathered during the build. Here, we need data from discovery about the backend. Hopefully/probably you can get this via the useStripes hooks and its discovery attribute, populated immediately after login. If discovery (or some other attribute; paw through https://folio-snapshot.dev.folio.org/settings/developer/stripes-inspector) doesn’t have what you need, let’s take a look at the functions that populate it and see if we just need to store more information during that initialization phase. In principle, though, I agree with you: we can probably get what we need without making a fresh query. If we can do that, then option 2 becomes irrelevant.

If we put all that aside and imagine that we did need an “override the react-query config” option, such an option really boils down to “maybe we are not doing a good job when we configure react-query”. If you have suggestions about more effective ways to configure react-query, I am all ears. If you have the time, I would love your assistance and am happy to follow your guidance since it seems you better understand/have more experience with react-query than I.

Again, my apologies for how I have handled this so far.

, It is because constants do not suit me that I want to use this hook, but it needs to be improved a little. I would like to discuss how we can improve it. I described 3 possible options in the description, from the ideas that I have.

The default caching from react-query doesn't work quite as it should for this case. Because the default values cacheTime: 5000 and staleTime: 0 don't prevent repeated requests. In the second and subsequent instances of useQuery with the same key, it will take the data immediately from the cache, but it won't prevent the second and subsequent requests to `/modules?full=true`.

In other words, I would like to update this hook and take values ​​from useModules or hardcode/allow configuration from outside staleTime > 0.

I have the capacity to help with the implementation, but I don't want to do PR until we agree on the solution

Does it makes sense for you?

the hook has no mechanism to get the data with a delay or cache it

, the hook uses react-query, which automatically provides caching.

Look, I understand your argument that few hard-coded constants are a fast and easy alternative to this heavyweight hook. That’s true and I can’t argue with you technically. My argument is that hard-coding module names runs counter to the deeply-ingrained principle of separating interfaces from the modules that implement them. IOW, maybe those constants are correct, or maybe they’re not, and since you know this – since you know they may not be correct – choosing to use them anyway is not a choice I would personally be comfortable with.