[native] Fix getStateFromNavigatorRoute calls
ClosedPublic
Actions

Authored by inka on Nov 23 2022, 2:12 AM.

Details

Reviewers

atul
tomek
kamil
bartek
ginsu
• przemek
ashoat

Commits

rCOMM7e50ae41e8e6: [native] Fix getStateFromNavigatorRoute calls

Summary

Linear issue: https://linear.app/comm/issue/ENG-2316/threadscreenpruner-problem
This code was trying to obtain route for Chat but was obtaining route for Profile. This way we can be sure that we are actually finding what we are looking for.

Test Plan

Run ios simulator, see that both on login screen and when user is logged in no errors arise. I also logged the value that is being returend to be sure this component is being rendered at all.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

inka created this revision.Nov 23 2022, 2:12 AM

Herald added a subscriber: ashoat. · View Herald TranscriptNov 23 2022, 2:12 AM

inka added inline comments.Nov 23 2022, 2:20 AM

native/chat/thread-screen-pruner.react.js
32 ↗	(On Diff #18746)	I changed the name because we are not really obtaining a route, but a `PossiblyStaleNavigationState` extracted from the route.

Harbormaster completed remote builds in B13680: Diff 18746.Nov 23 2022, 2:26 AM

inka requested review of this revision.Nov 23 2022, 2:26 AM

It's close!

native/chat/thread-screen-pruner.react.js

38–42 ↗

(On Diff #18746)

invariant does not take three params...

44–48 ↗

(On Diff #18746)

Can we create a helper in native/navigation/nav-selectors.js that does this?

function getChildRouteFromNavigatorRoute(parentRoute, childRouteName) {
  const parentState = getStateFromNavigatorRoute(parentRoute);
  const childRoute = parentState.routes.find(
    route => route.name === childRouteName,
  );
  invariant(childRoute, `parentRoute should contain route for ${childRouteName}`);
  return childRoute;
}

(Worth checking if such a helper already exists!)

This revision now requires changes to proceed.Nov 23 2022, 4:18 AM

Address Ashoat's comments (I didn't find a helper function that could be used here)

inka planned changes to this revision.Nov 23 2022, 6:19 AM

Harbormaster completed remote builds in B13697: Diff 18776.Nov 23 2022, 6:31 AM

I planned changes because something crashed locally, but it was a local cache problem.

In retrospect, a better place for this new helper is actually native/navigation/navigation-utils.js... sorry for changing the request, but could you move it there before landing? 😅