[ENDPOINT][TECH DEBT] Simplify the code to do history navigation in trusted apps (and other sections) #80143
Description
In trusted apps I've experimented with a custom hook for making navigation simpler:
There were some comments about it being not ideal:
The issue to be discussed solved - when implementing history navigation in trusted apps (and in other sections of endpoint management) for that matter we always have to perform couple of operations that are same all over the place:
- retrieve current location from Redux using
const location = useTrustedAppsSelector(getCurrentLocation); - change couple of parameters on it
const newLocation = { ...location, param: 'new value' } - generate URI out of it
const uri = getTrustedAppsListPath(newLocation); - call history push with that URI
hostory.push(uri);
Though it's not tremendously big chunk of code it repeats often and it makes every component that wants to do navigation directly have dependencies on useHistory (react routing), useTrustedAppsSelector (useSelector, react-redux), and location type. To avoid this boilerplate I've added the mentioned above hook. As mentioned it has couple of consequences:
- It is a new Hook/primitive which I unfortunately could not avoid as react's hooks are not a very composable primitive if you don't create custom hook (which in essence is just a function, same as useTrustedAppsSelector hook). It's debatable whether it is a big issue but it is something to write JSDoc for and developer needs to understand.
- Due to ESLint rules for hooks I had to disable checks for some lines, because I had to remove
callbackparameter from the list of dependencies. This is suboptimal. - This hook doesn't accept list of dependencies to be used for memoisation, neither it can reliably support that.
Here are some proposed solutions:
- Unless it's strongly insisted that we should not introduce any new primitives hooks for our convenience the new hook can be just documented and potentially moved to some common place (but then it needs to be generalised).
- For two other problems there is a middle ground solution - the callback passed to useTrustedAppsNavigateCallback could be wrapped with useCallback on the caller side. Something like:
useTrustedAppsNavigateCallback(
useCallback(({ page }) => ({
page_index: page.index,
page_size: page.size,
}), [])
)
Makes it a bit bulkier but solves the memoization problems and allows to remove disabling of ESLint check.