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: properly support index routes with a path in useResolvedPath #9486

Merged
merged 4 commits into from
Oct 21, 2022

Conversation

brophdawg11
Copy link
Contributor

While not a super common pattern, this was causing Form and useSubmit to use incorrect default actions when used in an index route with a path (more commonly generated from remix conventional routes):

<Route path="/">
  <Route path="foo" index={true} element={<Comp />} />
</Route>

A call to useFormAction inside Comp would incorrectly trim the foo route in getPathContributingMatches because it was an index route, and then it would generate a form action of /?index instead of /foo?index

@changeset-bot
Copy link

changeset-bot bot commented Oct 21, 2022

🦋 Changeset detected

Latest commit: ce47c0a

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-dom Patch
@remix-run/router Patch
react-router 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

@@ -19,6 +19,7 @@ import {
parsePath,
resolveTo,
warning,
UNSAFE_getPathContributingMatches as getPathContributingMatches,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the duped function and exported it as a private function from @remix-run/router

index === 0 ||
(!match.route.index &&
match.pathnameBase !== matches[index - 1].pathnameBase)
index === 0 || match.pathnameBase !== matches[index - 1].pathnameBase
Copy link
Contributor Author

@brophdawg11 brophdawg11 Oct 21, 2022

Choose a reason for hiding this comment

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

Don't look for match.route.index at all - just look to see if the pathnameBase changed

index === 0 ||
(!match.route.index &&
match.pathnameBase !== matches[index - 1].pathnameBase)
index === 0 || (match.route.path && match.route.path.length > 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

2 issues fixed here:

  • Looking at route.index doesn't work since index routes can have paths
  • Using pathnameBase doesn't work for nested splat routes since they don't add to pathnameBase, so instead we just look for the route do define a non-empty path value (also this means this becomes a function that we can use directly in remix since remix doesn't expose pathnameBase on matches)

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.

1 participant