App: [HOLD #11768] [$16000] Desktop - The Back ⌘[ and Forward ⌘] shortcuts keys are not working as expected in LHN

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


This issue has been split in two. This one is solely focused on the back/forward actions in LeftHandNav (LHN), the other https://github.com/Expensify/App/issues/8657 is focused on back/forward actions in the main chat window.

Action Performed:

  1. Open desktop app
  2. Navigate to several conversations
  3. Use the back ⌘[ and Forward ⌘] shortcuts

Expected Result:

Back ⌘[ and Forward ⌘] shortcuts should work and navigate through the previously navigated conversations and show correctly in LHN.

Actual Result:

Shortcuts are not working as expected.

Workaround:

User has to manually navigate through the conversations.

Platform:

Where is this issue occurring?

  • Desktop App

Version Number: 1.0.82-7

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation Unable to get a video atm. Will update with video later. Other issue for back/forward working correctly in the main chat window https://github.com/Expensify/App/issues/8657

Expensify/Expensify Issue URL:

View all open jobs on Upwork


From @mallenexpensify https://expensify.slack.com/archives/C01GTK53T8Q/p1628722621224600

The Back ⌘[ and Forward ⌘] shortcuts keys don’t work properly on Desktop Version 1.0.82-7

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Comments: 204 (134 by maintainers)

Most upvoted comments

I should mention that we overwrite the history on navigation custome actions so this is the important part.

Yes I added the code for it so I’m aware how it works 😄

Doubled price to $16000 Needed to create a new job https://www.upwork.com/jobs/~01294fe6c7ab1639a8

This is no more on hold. Waiting for proposals… and reviewing old ones.

I appreciate your proposal @murataka and I will take a look on Slack but the correct process is to post the proposal on Github.

I think we can incorporate it into the detailed design doc. My feeling about this issue is that the shortcuts never really worked as “expected” because, well, nobody ever promised that it would work a certain way. There are a lot of misunderstandings about how the memory history implemented in react-navigation works (especially with the drawer navigator in the mix).

There’s like 250+ comments I am not going to revisit here, but command + [ is the same thing as a browser back button. So if the browser back button works as expected on web and we switch to a simple stack navigator then I am pretty confident these shortcuts will “just work”.

I think the intended behavior should be explained in more detail now , it is hard to understand the intended behavior . is “navigation in the main chat view/window” , the RHN ? Do we need the navigation to be in sync with the navigation history and would the back and forward keys reflect the same list of navigated items , or would navigating change the order of the items on RHN, or should it have no affect when we navigate through the LHN on the RHN ? Do you think we need a search history as mentioned at https://github.com/Expensify/App/issues/4612#issuecomment-1025119790 ?

Of course they publish a release 1 day after we create our own fork 🙄

I am tackling the navigation issues one by one as it is possible one indirectly solves the other. Thanks for your patience.

Just share it if we can be helpful on any part of your review. I would suggest you to use the slack more actively while reviewing proposals, and dedicate to what you are currently doing. Sure you need to multitask for some reasons.

I am tackling the navigation issues one by one as it is possible one indirectly solves the other. Thanks for your patience.

I think i solve the main issue accourding to the comments i mentioned. So we wait for @parasharrajat and @NikkiWines to validat wich issue is the main and wich is the related one . Or both will be the main issue .

I mean Don’t navigate from drawer . Navigate from search page . That will reset the history . That’s the case not work .

@marcaaron @puneetlath I wanted to quickly confirm that we still we do commit to doing this in the react navigation project? I think largely the answer should be yes given how standard of a shortcut this is. However, I’m wavering a bit overall on the priority of keyboard actions overall. They all feel best done with a holistic, concerted push rather than piecemeal. Curious for your thoughts!

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

Currently, we are evaluating the status of navigation issues in our app. It is decided that fixing issues upstream is the best path forward. So if an issue can be fixed upstream then it would be preferred over others.

If you think that this is an issue with our repo, then please share a thorough explanation of it with your proposals.


Although there are two different issues for LHN and RHN, I consider RHN and LHN behavior to be tightly dependent. It might be possible that fixing the RHN issue requires changing the code in our custom logic (if this is an issue with our logic). Which could lead to a waste of resources on this issue. It is found over time that changing the custom Logic creates more problems. We would like to move away from that if possible.

For now, let’s focus on the issues first to understand them better before finding a solution. I would say it would be better to get the root cause of the RHN issue as well.

@ahmdshrif It seems that your solution is working but we are still not convinced if this is an issue with our logic. So we might take time before making any decision.

But the main point is the route is already pushed in history in the previous lines that’s why the report is changed to the clicked one.

@parasharrajat if you mean this line we only push routes equal length of previous history. but now we have a new route so the history should increase by one !! https://github.com/Expensify/App/blob/cc38098efa7c35daef2a7e7cb88872d16c952188/src/libs/Navigation/CustomActions.js#L100-L101

we should now add new history to browser. we can add it as a drawer if the state of the drawer changes (this case is a handle) otherwise, we should push it as a normal route. because that is what change

what do you think?

because we have a new route pushed to the drawer. if you use navigate() that will push automatically . since we use reset and add a new route we should add it to history also.

I will check it. But the main point is the route is already pushed in history in the previous lines that’s why the report is changed to the clicked one.

Thanks @ahmdshrif I will review this as soon as possible.

Proposal

the issue for LHN is different from RHN (search Screen) so this proposal is only for LHN

Problem :

https://user-images.githubusercontent.com/21364901/166839179-0c24bf24-3c57-4337-8647-40ddde7d95f5.mov

when we tap on LHN on user1 -> user2 -> user3 -> user4 -> user5 then click back or use cmd + [ to go back the app will navigate to user3 -> user1

so the history is skipping one user from history on every 2 navigation. we can see this on video.

Note here i add console.log(history) on this line and that what log on video https://github.com/Expensify/App/blob/cc38098efa7c35daef2a7e7cb88872d16c952188/src/libs/Navigation/CustomActions.js#L114

cause :

when we calculate the history to push it to reset state as shown here : https://github.com/Expensify/App/blob/cc38098efa7c35daef2a7e7cb88872d16c952188/src/libs/Navigation/CustomActions.js#L104-L113

we only push the drawer to history if we not have drawer history or we are not on drawer https://github.com/Expensify/App/blob/cc38098efa7c35daef2a7e7cb88872d16c952188/src/libs/Navigation/CustomActions.js#L106

when we navigate from user1 to user2

we are on drawer but we have drawer history so we will push drawer open to history .

but when we navigate form user2 to user3

we already have drawer history from last step so we will not push any thing to history . and will keep the history length equal to the previous history length !! but will replace all history with type: ‘route’ not drawer

https://github.com/Expensify/App/blob/cc38098efa7c35daef2a7e7cb88872d16c952188/src/libs/Navigation/CustomActions.js#L100-L101

again form user3 -> user4 push drawer
from user4 -> user5 not push any thing because we have drawer history and just replace old history with type route .

and loop will keep going as shown on video

Sloution :

we can push drawer t close as we do before when we were on the drawer. and we has drawer history with state open .

that is can be like the

-        const hasDrawerhistory = _.find(state.history || [], h => h.type === 'drawer');
+        const hasCloseDrawerhistory = _.find(state.history || [], h => h.type === 'drawer' && h.status === 'closed');
-        if (!hasDrawerhistory || currentState.type !== 'drawer') {
+       if (!hasCloseDrawerhistory || currentState.type !== 'drawer') {

or if you prefer not to push the drawer as open since it’s our default state we can push route. like this on line 113

        } else {
            history.push(screenRoute);
        }

Result

as shown in the video it’s now working fine.

https://user-images.githubusercontent.com/21364901/166842980-3a71eeee-72f9-45cf-a57e-81850dba66b3.mov

note I also tested the back button issue which was fixed by the code I edit on my proposal and it work fine.

cc : @parasharrajat

Thanks for sharing. I will run a few tests on weekends to observe the current state of the issue.

  • click Rajat in LHN * click Mitch in LHN * click Monte in LHN * ⌘[ goes to Mitch * ⌘[ goes to Rajat.

This is no longer true.

The current behavior on staging and production -

  • click Rajat in LHN
  • click Mitch in LHN
  • click Monte in LHN
  • ⌘[ goes to Rajat.

We’re skipping every other user. It used to work before, have we introduced some changes which broke this?

https://user-images.githubusercontent.com/29673073/159834008-5666c747-ec0d-47d0-9882-c11eba73bcac.mov

My bad. Forgot to leave a review for your proposal @ahmdshrif. Do you mind writing a single post explaining your approach to the problem? I saw your code but it is not clear enough.

  1. What do you think the problem is? Are you clear about the expected behavior here?
  2. What is causing this issue? And how it is doing that?
  3. Break down the issues into steps and try to solve each separately instead of giving a diff or code.
  4. List down all the navigation scenarios that are touched by the piece of code you are planning to change.
  5. Explain how your solution will work for each of those navigation scenarios.

I am trying to find solutions for other issues on that list. You can see the updates on those issues. If I find something that needs attention, I will open a discussion on slack and involve you.

Sure, I will be happy to help you with the CustomActions based on your proposed changes.

Update: This one is close. This is next on the list. I have already reviewed the initial proposals But I need to understand the new suggested changes.

Yeah, I am handling the issues in order (the order is not final). But the main reason is to fix issues one by one. Solutions can overlap. But I am also tracking proposals on each issue.

Yes, you can add [hold] if needed. I don’t have access to do that 😅 .

My proposal also need reviewing 😃

@parasharrajat what’s status here? Does @murataka’s proposal need reviewing? https://github.com/Expensify/App/issues/4612#issuecomment-1029745730

@kevinksullivan I’m out next week so auto-added you to keep 👀 on this til I’m back, then I’ll take back over.

Sure, I can do that.

The external bug is at last been included in

"@react-navigation/native": "6.0.8",

. I think that bug solved has many affects if not the root cause. I strongly suggest any reviewers to update package.json while reviewing any navigational issue, as this is external , and is a potential to cause false negatives .

Let me see what is the best thing to do. I will test your RHN solution and if that works it’s great. We can assign this issue.

The RHN solution will work only if you combine the upgrading the navigation library fix.

Thanks to @murataka, I am sure we can compensate for your efforts but I don’t consider that a complete solution to this issue.

Let me see what is the best thing to do. I will test your RHN solution and if that works it’s great. We can assign this issue. otherwise, we will find some way.

Please do not create a PR until requested to do so.

@parasharrajat , reactnative/navigation library has the proper solution merged now . but

package.json is still

"@react-navigation/native": "6.0.6",

and should be upgraded to

"@react-navigation/native": "6.0.7",

at least.

This has many impacts on other navigation related issues (and also some issues which seems to be unrelated ). I think update should be done asap , to prevent further related bug reports about navigation to getting whitescreen .

@parasharrajat , @ahmdshrif had much help on me focusing on the search (RHN) related issues. If possible make sure he also gets hired if my fixes are accepted.

since I spend many days working on changing the state in reset action, I can confirm that adding an index will not solve the RHN issue. but you can make a test also.

Without the react-navigation library fix, I totally understand you spending so much time on that. But that works after combining those fixes.

// Note: A current limitation with this is that navigating “back” won’t display the routes we have cleared out e.g. SearchPage and the history effectively gets “reset”.

@parasharrajat I think that you have some kind of distraction issues like me, in case you missed it, that seems to be the root cause of reset-like behavior mentioned above.

@ahmdshrif Thanks for the proposal. I will review that as well.

Correct, we can just focus on the reproduction steps here. This should fix that. All of the other issues have separate tasks/GH issues.

If you think they are separate, you can raise a slack thread. Others can help in deciding that.

I don’t think I’m the one to have a conclusion on that , please check the comments in CustomActions.js .

As any navigational issue will have an affect on how the history works, I think we should not relate all of those to this issue as if they are solving this issue. If related , I hope someone else makes a final conclusion on that 💭

To clarify the scope of this

  1. Report navigation order should be maintained on all cases. Whether the report is opened from LHN(left hand navigation) or RHN(right hand navigation).
  2. Also, when we open search page and open a report. navigating back should take you to previous report not the search page.

There are many navigational issues #7363 . It is possible that solution for one can solve others. This is known to us and must have been mentioned before. In that case we will discontinue other which is automatically solved. This is one of the reason, most of them are still open. We are looking for a concrete solution.

@murataka I tested your solution.

LHN report navigation seems to work better now. But navigation is not remembered for navigating from search page.

Let’s try fix that as well.


For general discussion, please keep it off the issue thread so that readers are not confused.

You better do a file sarch for useLinking.js

in App/node_modules folder.

Also , make sure you know how to reproduce the “id of undefined” error with the current version , so that you clearly understand what it solves ( navigate several items , back->back->back-forward->back-forward-forward->back->back … ).

You can use browser , as the error is affecting both native and browser .

@ahmdshrif , if you think it is related to this subject , I shall go open a bug report on slack (we need seperate search history ). If you think this is not related what you propose , you should report it ( that bug we just found ) on slack channel I think .

thanks for sharing that. @parasharrajat will test this so no need to retest from my side . I just think there is 2 issue and we need both pr to solve them . that is what I mean in my comment. if your proposal solves the 2 cases I mention it will be the only accepted one I agree with this.

A note to who want to test :

As copying and pasting the file is not the proper way of testing it, and while doing the back and forward uses caching when possible , be sure to repeat the test on maybe on an other browser or be sure to clear app cache. Thanks for anyone testing it also , as soon as you share any information about the bug . But as suggested above , there seems to be no urgency. I am just trying to be helpful, and don’t think have to solve the bug at all , but prove of the core problem was possibly already enough for the proposal. I hope all is clear , and yes I shall continue to work on it as much as I want . Agains thanks for any additional information .

import { findFocusedRoute, getActionFromState as getActionFromStateDefault, getPathFromState as getPathFromStateDefault, getStateFromPath as getStateFromPathDefault } from '@react-navigation/core';
import { nanoid } from 'nanoid/non-secure';
import * as React from 'react';
import ServerContext from './ServerContext';

const createMemoryHistory = () => {
  let index = 0;
  let items = []; // Pending callbacks for `history.go(n)`
  // We might modify the callback stored if it was interrupted, so we have a ref to identify it

  const pending = [];

  const interrupt = () => {
    // If another history operation was performed we need to interrupt existing ones
    // This makes sure that calls such as `history.replace` after `history.go` don't happen
    // Since otherwise it won't be correct if something else has changed
    pending.forEach(it => {
      const cb = it.cb;

      it.cb = () => cb(true);
    });
  };

  const history = {
    get index() {
      var _window$history$state;

      // We store an id in the state instead of an index
      // Index could get out of sync with in-memory values if page reloads
      const id = (_window$history$state = window.history.state) === null || _window$history$state === void 0 ? void 0 : _window$history$state.id;

      if (id) {
        const index = items.findIndex(item => item&&item.id === id);
        return index > -1 ? index : 0;
      }

      return 0;
    },

    get(index) {
      return items[index];
    },

    backIndex({
      path
    }) {
      // We need to find the index from the element before current to get closest path to go back to
      for (let i = index - 1; i >= 0; i--) {
        const item = items[i];

        if (item.path === path) {
          return i;
        }
      }

      return -1;
    },

    push({
      path,
      state
    }) {
      interrupt();
      const id = nanoid(); // When a new entry is pushed, all the existing entries after index will be inaccessible
      // So we remove any existing entries after the current index to clean them up

      items = items.slice(0, index + 1);
      items.push({
        path,
        state,
        id
      });
      index = items.length - 1; // We pass empty string for title because it's ignored in all browsers except safari
      // We don't store state object in history.state because:
      // - browsers have limits on how big it can be, and we don't control the size
      // - while not recommended, there could be non-serializable data in state

      window.history.pushState({
        id
      }, '', path);
    },

    replace({
      path,
      state
    }) {
      var _window$history$state2, _window$history$state3;

      interrupt();
      const id = (_window$history$state2 = (_window$history$state3 = window.history.state) === null || _window$history$state3 === void 0 ? void 0 : _window$history$state3.id) !== null && _window$history$state2 !== void 0 ? _window$history$state2 : nanoid();

      if (!items.length || items.findIndex(item =>item&& item.id === id) < 0) {
        // There are two scenarios for creating an array with only one history record:
        // - When loaded id not found in the items array, this function by default will replace
        //   the first item. We need to keep only the new updated object, otherwise it will break
        //   the page when navigating forward in history.
        // - This is the first time any state modifications are done
        //   So we need to push the entry as there's nothing to replace
        items = [{
          path,
          state,
          id
        }];
      } else {
        items [0]={
          path,
          state,
          id
        };
      }

      window.history.replaceState({
        id
      }, '', path);
    },

    // `history.go(n)` is asynchronous, there are couple of things to keep in mind:
    // - it won't do anything if we can't go `n` steps, the `popstate` event won't fire.
    // - each `history.go(n)` call will trigger a separate `popstate` event with correct location.
    // - the `popstate` event fires before the next frame after calling `history.go(n)`.
    // This method differs from `history.go(n)` in the sense that it'll go back as many steps it can.
    go(n) {
      interrupt();

      if (n > 0) {
        // We shouldn't go forward more than available index
        n = Math.min(n, items.length - 1);
      } else if (n < 0) {
        // We shouldn't go back more than the 0 index
        // Otherwise we'll exit the page
        n = index + n < 0 ? -index : n;
      }

      if (n === 0) {
        return;
      }

      index += n; // When we call `history.go`, `popstate` will fire when there's history to go back to
      // So we need to somehow handle following cases:
      // - There's history to go back, `history.go` is called, and `popstate` fires
      // - `history.go` is called multiple times, we need to resolve on respective `popstate`
      // - No history to go back, but `history.go` was called, browser has no API to detect it

      return new Promise((resolve, reject) => {
        const done = interrupted => {
          clearTimeout(timer);

          if (interrupted) {
            reject(new Error('History was changed during navigation.'));
            return;
          } // There seems to be a bug in Chrome regarding updating the title
          // If we set a title just before calling `history.go`, the title gets lost
          // However the value of `document.title` is still what we set it to
          // It's just not displayed in the tab bar
          // To update the tab bar, we need to reset the title to something else first (e.g. '')
          // And set the title to what it was before so it gets applied
          // It won't work without setting it to empty string coz otherwise title isn't changing
          // Which means that the browser won't do anything after setting the title


          const {
            title
          } = window.document;
          window.document.title = '';
          window.document.title = title;
          resolve();
        };

        pending.push({
          ref: done,
          cb: done
        }); // If navigation didn't happen within 100ms, assume that it won't happen
        // This may not be accurate, but hopefully it won't take so much time
        // In Chrome, navigation seems to happen instantly in next microtask
        // But on Firefox, it seems to take much longer, around 50ms from our testing
        // We're using a hacky timeout since there doesn't seem to be way to know for sure

        const timer = setTimeout(() => {
          const index = pending.findIndex(it => it.ref === done);

          if (index > -1) {
            pending[index].cb();
            pending.splice(index, 1);
          }
        }, 100);

        const onPopState = () => {
          var _window$history$state4;

          const id = (_window$history$state4 = window.history.state) === null || _window$history$state4 === void 0 ? void 0 : _window$history$state4.id;
          const currentIndex = items.findIndex(item =>item&& item.id === id); // Fix createMemoryHistory.index variable's value
          // as it may go out of sync when navigating in the browser.

          index = Math.max(currentIndex, 0);
          const last = pending.pop();
          window.removeEventListener('popstate', onPopState);
          last === null || last === void 0 ? void 0 : last.cb();
        };

        window.addEventListener('popstate', onPopState);
        window.history.go(n);
      });
    },

    // The `popstate` event is triggered when history changes, except `pushState` and `replaceState`
    // If we call `history.go(n)` ourselves, we don't want it to trigger the listener
    // Here we normalize it so that only external changes (e.g. user pressing back/forward) trigger the listener
    listen(listener) {
      const onPopState = () => {
        if (pending.length) {
          // This was triggered by `history.go(n)`, we shouldn't call the listener
          return;
        }

        listener();
      };

      window.addEventListener('popstate', onPopState);
      return () => window.removeEventListener('popstate', onPopState);
    }

  };
  return history;
};
/**
 * Find the matching navigation state that changed between 2 navigation states
 * e.g.: a -> b -> c -> d and a -> b -> c -> e -> f, if history in b changed, b is the matching state
 */


const findMatchingState = (a, b) => {
  if (a === undefined || b === undefined || a.key !== b.key) {
    return [undefined, undefined];
  } // Tab and drawer will have `history` property, but stack will have history in `routes`


  const aHistoryLength = a.history ? a.history.length : a.routes.length;
  const bHistoryLength = b.history ? b.history.length : b.routes.length;
  const aRoute = a.routes[a.index];
  const bRoute = b.routes[b.index];
  const aChildState = aRoute.state;
  const bChildState = bRoute.state; // Stop here if this is the state object that changed:
  // - history length is different
  // - focused routes are different
  // - one of them doesn't have child state
  // - child state keys are different

  if (aHistoryLength !== bHistoryLength || aRoute.key !== bRoute.key || aChildState === undefined || bChildState === undefined || aChildState.key !== bChildState.key) {
    return [a, b];
  }

  return findMatchingState(aChildState, bChildState);
};
/**
 * Run async function in series as it's called.
 */


const series = cb => {
  // Whether we're currently handling a callback
  let handling = false;
  let queue = [];

  const callback = async () => {
    try {
      if (handling) {
        // If we're currently handling a previous event, wait before handling this one
        // Add the callback to the beginning of the queue
        queue.unshift(callback);
        return;
      }

      handling = true;
      await cb();
    } finally {
      handling = false;

      if (queue.length) {
        // If we have queued items, handle the last one
        const last = queue.pop();
        last === null || last === void 0 ? void 0 : last();
      }
    }
  };

  return callback;
};

let linkingHandlers = [];
export default function useLinking(ref, {
  independent,
  enabled = true,
  config,
  getStateFromPath = getStateFromPathDefault,
  getPathFromState = getPathFromStateDefault,
  getActionFromState = getActionFromStateDefault
}) {
  React.useEffect(() => {
    if (process.env.NODE_ENV === 'production') {
      return undefined;
    }

    if (independent) {
      return undefined;
    }

    if (enabled !== false && linkingHandlers.length) {
      console.error(['Looks like you have configured linking in multiple places. This is likely an error since deep links should only be handled in one place to avoid conflicts. Make sure that:', "- You don't have multiple NavigationContainers in the app each with 'linking' enabled", '- Only a single instance of the root component is rendered'].join('\n').trim());
    }

    const handler = Symbol();

    if (enabled !== false) {
      linkingHandlers.push(handler);
    }

    return () => {
      const index = linkingHandlers.indexOf(handler);

      if (index > -1) {
        linkingHandlers.splice(index, 1);
      }
    };
  }, [enabled, independent]);
  const [history] = React.useState(createMemoryHistory); // We store these options in ref to avoid re-creating getInitialState and re-subscribing listeners
  // This lets user avoid wrapping the items in `React.useCallback` or `React.useMemo`
  // Not re-creating `getInitialState` is important coz it makes it easier for the user to use in an effect

  const enabledRef = React.useRef(enabled);
  const configRef = React.useRef(config);
  const getStateFromPathRef = React.useRef(getStateFromPath);
  const getPathFromStateRef = React.useRef(getPathFromState);
  const getActionFromStateRef = React.useRef(getActionFromState);
  React.useEffect(() => {
    enabledRef.current = enabled;
    configRef.current = config;
    getStateFromPathRef.current = getStateFromPath;
    getPathFromStateRef.current = getPathFromState;
    getActionFromStateRef.current = getActionFromState;
  });
  const server = React.useContext(ServerContext);
  const getInitialState = React.useCallback(() => {
    let value;

    if (enabledRef.current) {
      var _server$location;

      const location = (_server$location = server === null || server === void 0 ? void 0 : server.location) !== null && _server$location !== void 0 ? _server$location : typeof window !== 'undefined' ? window.location : undefined;
      const path = location ? location.pathname + location.search : undefined;

      if (path) {
        value = getStateFromPathRef.current(path, configRef.current);
      }
    }

    const thenable = {
      then(onfulfilled) {
        return Promise.resolve(onfulfilled ? onfulfilled(value) : value);
      },

      catch() {
        return thenable;
      }

    };
    return thenable; // eslint-disable-next-line react-hooks/exhaustive-deps
  }, []);
  const previousIndexRef = React.useRef(undefined);
  const previousStateRef = React.useRef(undefined);
  const pendingPopStatePathRef = React.useRef(undefined);
  React.useEffect(() => {
    previousIndexRef.current = history.index;
    return history.listen(() => {
      var _previousIndexRef$cur;

      const navigation = ref.current;

      if (!navigation || !enabled) {
        return;
      }

      const path = location.pathname + location.search;
      const index = history.index;
      const previousIndex = (_previousIndexRef$cur = previousIndexRef.current) !== null && _previousIndexRef$cur !== void 0 ? _previousIndexRef$cur : 0;
      previousIndexRef.current = index;
      pendingPopStatePathRef.current = path; // When browser back/forward is clicked, we first need to check if state object for this index exists
      // If it does we'll reset to that state object
      // Otherwise, we'll handle it like a regular deep link

      const record = history.get(index);

      if ((record === null || record === void 0 ? void 0 : record.path) === path && record !== null && record !== void 0 && record.state) {
        navigation.resetRoot(record.state);
        return;
      }

      const state = getStateFromPathRef.current(path, configRef.current); // We should only dispatch an action when going forward
      // Otherwise the action will likely add items to history, which would mess things up

      if (state) {
        // Make sure that the routes in the state exist in the root navigator
        // Otherwise there's an error in the linking configuration
        const rootState = navigation.getRootState();

        if (state.routes.some(r => !(rootState !== null && rootState !== void 0 && rootState.routeNames.includes(r.name)))) {
          console.warn("The navigation state parsed from the URL contains routes not present in the root navigator. This usually means that the linking configuration doesn't match the navigation structure. See https://reactnavigation.org/docs/configuring-links for more details on how to specify a linking configuration.");
          return;
        }

        if (index > previousIndex) {
          const action = getActionFromStateRef.current(state, configRef.current);

          if (action !== undefined) {
            try {
              navigation.dispatch(action);
            } catch (e) {
              // Ignore any errors from deep linking.
              // This could happen in case of malformed links, navigation object not being initialized etc.
              console.warn(`An error occurred when trying to handle the link '${path}': ${e.message}`);
            }
          } else {
            navigation.resetRoot(state);
          }
        } else {
          navigation.resetRoot(state);
        }
      } else {
        // if current path didn't return any state, we should revert to initial state
        navigation.resetRoot(state);
      }
    });
  }, [enabled, history, ref]);
  React.useEffect(() => {
    var _ref$current;

    if (!enabled) {
      return;
    }

    if (ref.current) {
      // We need to record the current metadata on the first render if they aren't set
      // This will allow the initial state to be in the history entry
      const state = ref.current.getRootState();

      if (state) {
        var _route$path;

        const route = findFocusedRoute(state);
        const path = (_route$path = route === null || route === void 0 ? void 0 : route.path) !== null && _route$path !== void 0 ? _route$path : getPathFromStateRef.current(state, configRef.current);

        if (previousStateRef.current === undefined) {
          previousStateRef.current = state;
        }

        history.replace({
          path,
          state
        });
      }
    }

    const onStateChange = async () => {
      var _route$path2;

      const navigation = ref.current;

      if (!navigation || !enabled) {
        return;
      }

      const previousState = previousStateRef.current;
      const state = navigation.getRootState();
      const pendingPath = pendingPopStatePathRef.current;
      const route = findFocusedRoute(state);
      const path = (_route$path2 = route === null || route === void 0 ? void 0 : route.path) !== null && _route$path2 !== void 0 ? _route$path2 : getPathFromStateRef.current(state, configRef.current);
      previousStateRef.current = state;
      pendingPopStatePathRef.current = undefined; // To detect the kind of state change, we need to:
      // - Find the common focused navigation state in previous and current state
      // - If only the route keys changed, compare history/routes.length to check if we go back/forward/replace
      // - If no common focused navigation state found, it's a replace

      const [previousFocusedState, focusedState] = findMatchingState(previousState, state);

      if (previousFocusedState && focusedState && // We should only handle push/pop if path changed from what was in last `popstate`
      // Otherwise it's likely a change triggered by `popstate`
      path !== pendingPath) {
        const historyDelta = (focusedState.history ? focusedState.history.length : focusedState.routes.length) - (previousFocusedState.history ? previousFocusedState.history.length : previousFocusedState.routes.length);

        if (historyDelta > 0) {
          // If history length is increased, we should pushState
          // Note that path might not actually change here, for example, drawer open should pushState
          history.push({
            path,
            state
          });
        } else if (historyDelta < 0) {
          // If history length is decreased, i.e. entries were removed, we want to go back
          const nextIndex = history.backIndex({
            path
          });
          const currentIndex = history.index;

          try {
            if (nextIndex !== -1 && nextIndex < currentIndex) {
              // An existing entry for this path exists and it's less than current index, go back to that
              await history.go(nextIndex - currentIndex);
            } else {
              // We couldn't find an existing entry to go back to, so we'll go back by the delta
              // This won't be correct if multiple routes were pushed in one go before
              // Usually this shouldn't happen and this is a fallback for that
              await history.go(historyDelta);
            } // Store the updated state as well as fix the path if incorrect


            history.replace({
              path,
              state
            });
          } catch (e) {// The navigation was interrupted
          }
        } else {
          // If history length is unchanged, we want to replaceState
          history.replace({
            path,
            state
          });
        }
      } else {
        // If no common navigation state was found, assume it's a replace
        // This would happen if the user did a reset/conditionally changed navigators
        history.replace({
          path,
          state
        });
      }
    }; // We debounce onStateChange coz we don't want multiple state changes to be handled at one time
    // This could happen since `history.go(n)` is asynchronous
    // If `pushState` or `replaceState` were called before `history.go(n)` completes, it'll mess stuff up


    return (_ref$current = ref.current) === null || _ref$current === void 0 ? void 0 : _ref$current.addListener('state', series(onStateChange));
  });
  return {
    getInitialState
  };
}
//# sourceMappingURL=useLinking.js.map

I shall have a proper solution in a few hours for sure.

Have you tested the fix on our repo? Does it fix the issue here? Also, can I have a test branch to test with? It is completely the third party so it would make more sense.

Yes , this is the case , but I did not hesitate to fix it on that repo , with the expectation I can be helpful a bit to @satya164 the maintainer of the repo… not finished yet, hope soon I will.

Have you tested the fix on our repo? Does it fix the issue here? Also, can I have a test branch to test with? It is completely the third party so it would make more sense.

As expected , the workaround will possibly cause issues, yes it seems to work at our repo , but I will fix it as a proper solution , and sleep comfortably.

for future reference, @satya164 is the primary maintainer of react navigation and is available for hire to work on these types of issues

Could you please do more research and gather information before we discuss a solution here?

  1. we use react-navigation.
  2. history.forward() === browserWindow.webContents.goForward.

@parasharrajat can you do more testing based on my comment above and see what you find? I’m still on Version 1.1.18-3

Oops, forgot to check it due to hold on 6207 but I will look into it from now. It could take a day or two.

Updated are posted on #6207 a couple of days back. Waiting for the response there.

I will do this.

so maybe retest this issue and try out default behaviour

Ok so, who should do this? 😄

Looks like the review for https://github.com/Expensify/App/pull/6207 is on hold while Marc is OOO, so seemsl like we should expect any progress on this to be delayed until next week

Ok. So I am looking into this. I think the changes I did https://github.com/Expensify/App/pull/6207 here are also needed for this to work. The main issue is that history is not created correctly when we use chat switcher to change the chats. I will test it on that branch and share the updates shortly.

N6-hold no longer applies - @parasharrajat you should feel free to work on this now.

I still have to dig more into this. I will update soon

@MitchExpensify I think this needs Exported label so that an engineer can review proposals.