Page MenuHomePhabricator

[native] Fix getStateFromNavigatorRoute calls
ClosedPublic

Authored by inka on Nov 23 2022, 2:12 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 19, 3:35 PM
Unknown Object (File)
Tue, Nov 19, 3:35 PM
Unknown Object (File)
Tue, Nov 19, 3:35 PM
Unknown Object (File)
Tue, Nov 19, 3:35 PM
Unknown Object (File)
Tue, Nov 19, 3:19 PM
Unknown Object (File)
Tue, Nov 19, 3:15 PM
Unknown Object (File)
Wed, Oct 30, 7:08 AM
Unknown Object (File)
Oct 17 2024, 9:01 AM
Subscribers

Details

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
Branch
inka/chatRoute_fix
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

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

I changed the name because we are not really obtaining a route, but a PossiblyStaleNavigationState extracted from the route.

inka requested review of this revision.Nov 23 2022, 2:26 AM
ashoat requested changes to this revision.Nov 23 2022, 4:18 AM

It's close!

native/chat/thread-screen-pruner.react.js
38–42

invariant does not take three params...

44–48

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
inka requested review of this revision.Nov 23 2022, 7:26 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? 😅

native/navigation/nav-selectors.js
281 ↗(On Diff #18776)

Not sure why it's missing in this file, but in general we should have an extra newline before the export declaration

This revision is now accepted and ready to land.Nov 23 2022, 7:33 AM

Move getChildRouteFromNavigatorRoute to navigation-utils.js