Chore: share navigate-back function between namespaces

[seanstrom]

Problem

Summary

• It seems we have many usages of (rf/dispatch [:navigate-back]) in the codebase.
  • Sometimes they're defined as anonymous functions #(rf/dispatch [:navigate-back])
  • Sometimes they're defined as static functions:

    (defn navigate-back
      []
      (rf/dispatch [:navigate-back]))

Anonymous Functions

Usages #(rf/dispatch [:navigate-back]) without a use-callback or use-memo can lead to more re-renders because of the anonymous function being passed as a prop which invalidates the React shouldComponentUpdate (aka React.memo) logic used by Reagent.

It's also worth mentioning that using use-callback or use-memo would be unnecessary since the expression #(rf/dispatch [:navigate-back]) does not use any variables or scope from a component. Which means it can be safely refactored into a static function inside the component namespace.

This means usages of #(rf/dispatch [:navigate-back]) would become (defn navigate-back [] ...) in practice.

Static functions

Using static functions is preferred because they create stable function references for components to pass around as props. Which leads to potentially less unnecessary re-renders because we're always passing the same function reference around.

However, defining many (defn navigate-back [] ...) functions in many different namespaces can have other drawbacks. For example, if we want refactor the app's naming of :navigate-back to something like :navigation/back or ::navigation/back, we'll need to go to each local definition of (defn navigate-back [] ...) and update the keywords.

Ideally, we would have a way to share the definition of (defn navigate-back [] ...) so that we can improve maintenance of the code and make refactors simpler. For example, if we share a static function from a namespace, we can easily find all the usages of the function with familiar IDE tooling. We can also easily rename the function and the event keyword in one/two places, which is much easier than needing to find all usages of the :navigate-back keyword being passed to a dispatch function.

• Note: the dispatch function can be aliased, so we couldn't assume it's named dispatch when searching.
• Note: we manage the naming of the event keyword in one/two places depending on how we define the Re-Frame event handler and the dispatch helper.

Implementation

One potential solution is to revise the code to share a navigate-back function from a navigation or utility namespace. This was already being done from an events namespace here:

status-mobile/src/status_im/navigation/events.cljs

Lines 47 to 50 in 4f0a49f

	(rf/defn navigate-back
	{:events [:navigate-back]}
	[cofx]
	(shell.events/shell-navigate-back cofx nil))

But I'm unsure whether we want more namespaces depending on the status-im.navigation.events namespace. If possible, we would decide whether to use this namespace or define a separate namespace for common dispatch functions. Though the downside to creating a separate namespace is that renaming the event name requires us to change it in two places.

[Parveshdhull]

If possible, we would decide whether to use this namespace or define a separate namespace for common dispatch functions

Thank you @seanstrom for creating this issue. As navigation.events is namespace for navigation related events, I think having a separate namespace will be better. In that way we also prevent cyclic dependency issue that might occur for events namespace.