Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

createMemoryHistory always keeps current hash even if replacement has a hash #12001

Open
3 of 11 tasks
jmbldwn opened this issue May 22, 2024 · 4 comments
Open
3 of 11 tasks

Comments

@jmbldwn
Copy link

jmbldwn commented May 22, 2024

Current behavior

with url at https://host/#/foo/bar, calling history.replace({path: '#/foo/baz', ...}) sets the new url to https://host/#/foo/baz/#/foo/bar.

This conflicts with using react-navigation with a 'hash router' method.

Source of the issue is this code, which intentionally adds location.hash to the path.

    replace({ path, state }: { path: string; state: NavigationState }) {
      interrupt();

      const id = window.history.state?.id ?? nanoid();

      // Need to keep the hash part of the path if there was no previous history entry
      // or the previous history entry had the same path
      let pathWithHash = path;

      if (!items.length || items.findIndex((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
        pathWithHash = pathWithHash + location.hash;
        items = [{ path: pathWithHash, state, id }];
        index = 0;
      } else {
        if (items[index].path === path) {
          pathWithHash = pathWithHash + location.hash;
        }
        items[index] = { path, state, id };
      }

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

Expected behavior

The location url could/should drop location.hash if the new path has a hash in it, because there's no world where we'd want to have two hash parts.

resulting location for above case would be: https://host/#/foo/baz

Reproduction

https://snack.expo.dev/@jmbldwn/react-navigation-has-repro

Platform

  • Android
  • iOS
  • Web
  • Windows
  • MacOS

Packages

  • @react-navigation/bottom-tabs
  • @react-navigation/drawer
  • @react-navigation/material-top-tabs
  • @react-navigation/stack
  • @react-navigation/native-stack
  • react-native-tab-view

Environment

package version
"@react-navigation/native": "^6.1.17",
"@react-navigation/native-stack": "^6.9.26",
"@react-navigation/stack": "^6.3.29",
"react-native": "0.73.6",
"react-native-gesture-handler": "~2.14.0",
"react-native-reanimated": "~3.6.2",
"react-native-safe-area-context": "4.8.2",
"react-native-screens": "~3.29.0",
"react-native-web": "~0.19.6"
"expo": "^50.0.0",

node 16.20.2
yarn 3.8.2

Copy link

The versions mentioned in the issue for the following packages differ from the latest versions on npm:

  • @react-navigation/stack (found: 6.3.16, latest: 6.3.29)

Can you verify that the issue still exists after upgrading to the latest versions of these packages?

@jmbldwn
Copy link
Author

jmbldwn commented May 22, 2024

Still working on the repro, but I think the code from createMemoryHistory.tsx shows why this is happening.

@jmbldwn
Copy link
Author

jmbldwn commented May 22, 2024

The versions mentioned in the issue for the following packages differ from the latest versions on npm:

  • @react-navigation/stack (found: 6.3.16, latest: 6.3.29)

Can you verify that the issue still exists after upgrading to the latest versions of these packages?

Yes. I was already using 6.3.29 anyway (semver)

@jmbldwn
Copy link
Author

jmbldwn commented May 22, 2024

I'll add that there is a workaround, but it's a bit of a hack. when I extract the hash part of the url on first page load, I blow away the current hash so this code can't reappend it. Not ideal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant