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(react-router): take from into account when navigating with params or layouts #1721

Merged
merged 3 commits into from
Jun 10, 2024

Conversation

chorobin
Copy link
Contributor

@chorobin chorobin commented Jun 6, 2024

  • Use flatRoutes so we can select branches even if they have index routes
  • Do not override from redirect preloads. Users should specify this manually
  • Ignore from in declarative route masks when building the location
  • Add tests to cover bugs encountered

…ckout

- routesByPath doesn't include branches, they are deduped with index routes
  therefor I am using flatRoutes
- preload redirects should not include from automatically as it should be explicit
- declarative route masking should be pass from because from for route masking does
  not assert a location
Copy link

nx-cloud bot commented Jun 6, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit a982e76. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 4 targets

Sent with 💌 from NxCloud.

@chorobin chorobin changed the title fix: take from into acount for routes with params and layouts fix: take from into account when navigating with params or layouts Jun 6, 2024
@schiller-manuel schiller-manuel marked this pull request as ready for review June 7, 2024 15:57
@@ -1773,8 +1786,7 @@ export class Router<
preload: !!preload,
context: parentContext,
location,
navigate: (opts: any) =>
this.navigate({ ...opts, from: match.pathname }),
navigate: (opts: any) => this.navigate({ ...opts }),
Copy link
Member

Choose a reason for hiding this comment

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

Could avoid the spread here yea?

navigate: (opts: any) => this.navigate(opts as any)

packages/react-router/src/router.ts Show resolved Hide resolved
packages/react-router/src/useNavigate.tsx Show resolved Hide resolved
@SeanCassiere SeanCassiere changed the title fix: take from into account when navigating with params or layouts fix(react-router): take from into account when navigating with params or layouts Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants