Skip to content

Fix Next.js Pages Router pathname in AppProvider - #4657

Merged
apedroferreira merged 3 commits into
mui:masterfrom
apedroferreira:fix-pages-router-pathname
Feb 4, 2025
Merged

Fix Next.js Pages Router pathname in AppProvider#4657
apedroferreira merged 3 commits into
mui:masterfrom
apedroferreira:fix-pages-router-pathname

Conversation

@apedroferreira

Copy link
Copy Markdown
Collaborator

Use pathname in Next.js Pages Router adapter.

Closes #4645 (comment)

@apedroferreira apedroferreira added the type: bug It doesn't behave as expected. label Feb 4, 2025
@apedroferreira
apedroferreira requested a review from Janpot February 4, 2025 11:09
@apedroferreira apedroferreira self-assigned this Feb 4, 2025
*/
export function NextAppProviderPages(props: AppProviderProps) {
const { push, replace, asPath, query } = useRouter();
const { push, replace, pathname, query } = useRouter();

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@Janpot is this ok or is there a reason why you were using asPath?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is there a reason why you were using asPath?

only short-sightedness 🙂

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I believe the reason for using asPath is that pathname has a different form when running a nextjs app with skipMiddlewareUrlNormalize: true option for dynamic routes — pathname would look like /route/[param] for route /route/[param].ts. I believe the older solution of asPath.split('?')[0] worked better.

@apedroferreira apedroferreira Mar 10, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Hi, yes, that's actually something I reverted recently in the CRUD component PR as I noticed some issues with using pathname.
So in this upcoming release it will be asPath.split('?')[0] (we also didn't have the split with "?" before)

@mui-bot

mui-bot commented Feb 4, 2025

Copy link
Copy Markdown

Netlify deploy preview

https://deploy-preview-4657--mui-toolpad-docs.netlify.app/

Generated by 🚫 dangerJS against f0ee81a

@apedroferreira
apedroferreira enabled auto-merge (squash) February 4, 2025 11:20
@apedroferreira
apedroferreira merged commit 3a3821f into mui:master Feb 4, 2025
@apedroferreira
apedroferreira deleted the fix-pages-router-pathname branch February 4, 2025 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug It doesn't behave as expected.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NextJS Page Router - navigation item is not matching if url has query params

4 participants