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

Fix other code paths for resolveTo from a splat route #11045

Merged
merged 2 commits into from
Nov 20, 2023

Conversation

brophdawg11
Copy link
Contributor

Follow up to #10983 to handle other code paths calling getPathContributingMatches/resolveTo

Closes #11038

Copy link

changeset-bot bot commented Nov 20, 2023

🦋 Changeset detected

Latest commit: 2b06e27

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
react-router Patch
@remix-run/router Patch
react-router-dom Patch
react-router-dom-v5-compat Patch
react-router-native Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

idx === matches.length - 1 ? match.pathname : match.pathnameBase
)
);
let routePathnamesJson = JSON.stringify(getResolveToMatches(matches));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change on line 319 was only made in the one code path before. To avoid this same mistake in the future, we're now using a utility getResolveToMatches to share this logic.

@@ -138,7 +138,7 @@ describe("path resolution", () => {
path: "*",
},
],
"/foo",
"/foo/bar",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to #10983, we had a test here asserting the previously buggy behavior. A link to "." from a splat should stay on the current location, not route up to the parent

@brophdawg11 brophdawg11 merged commit 58d421f into dev Nov 20, 2023
3 checks passed
@brophdawg11 brophdawg11 deleted the brophdawg11/resolve-to-matches branch November 20, 2023 20:05
Copy link
Contributor

🤖 Hello there,

We just published version 6.20.0-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Copy link
Contributor

🤖 Hello there,

We just published version 6.20.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

jonathanpruvost pushed a commit to concordnow/react-router that referenced this pull request Nov 28, 2023
* Fix other code paths for resolveTo from a splat route

* Bump bundle
brophdawg11 added a commit that referenced this pull request Dec 1, 2023
brophdawg11 added a commit that referenced this pull request Dec 1, 2023
* Revert "Fix other code paths for resolveTo from a splat route (#11045)"

This reverts commit 58d421f.

* Revert "Add additional test case for #10983"

This reverts commit f320378.

* Revert "Fix issues with useFormAction/useResolvedPath for dot paths in param/splat routes (#10983)"

This reverts commit fe066bd.

* Fix test

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

Successfully merging this pull request may close these issues.

2 participants