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.

Sign up for GitHub

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

Jump to bottom

Update examples/active-class-name #34205

Merged
merged 4 commits into from
Feb 11, 2022
Merged

Update examples/active-class-name #34205

merged 4 commits into from
Feb 11, 2022

Conversation

xhoantran
Copy link
Contributor

@xhoantran xhoantran commented Feb 10, 2022 •
edited by ijjk
Loading

Description

According to Next.js useRouter documentation:

  • asPath: The path (including the query) shown in the browser without the configured basePath or locale.
  • pathname: Current route. That is the path of the page in /pages, the configured basePath or locale is not included.

asPath should not be used as the props of components. There are many cases that asPath not working as expected. For example:

  • asPath is different on server-side and client-side.
  • asPath can contains id and query.

Suggestion

  • Warning the use of asPath can lead to the conflict of client and server-side.
  • Update useRouter document.

Bug

  • Related issues linked using fixes #number

Fixes: #34144
Fixes: #34016
Fixes: #34197

@ijjk ijjk added the examples Issue/PR related to examples label Feb 10, 2022
@xhoantran xhoantran closed this Feb 10, 2022
@xhoantran xhoantran deleted the update-active-class-name branch February 10, 2022 22:42
@xhoantran xhoantran restored the update-active-class-name branch February 10, 2022 23:10
@xhoantran xhoantran reopened this Feb 10, 2022
@xhoantran
Copy link
Contributor Author

@ijjk Can you help me review that?

ijjk
ijjk approved these changes Feb 11, 2022
View reviewed changes
Copy link
Member

@ijjk ijjk left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the PR!

@kodiakhq kodiakhq bot merged commit a38e144 into vercel:canary Feb 11, 2022
@xhoantran xhoantran deleted the update-active-class-name branch February 11, 2022 00:56
@xhoantran xhoantran mentioned this pull request Feb 25, 2022
Closed
1 task
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ijjk ijjk ijjk approved these changes

@timneutkens timneutkens Awaiting requested review from timneutkens

@shuding shuding Awaiting requested review from shuding

@leerob leerob Awaiting requested review from leerob

@steven-tey steven-tey Awaiting requested review from steven-tey

Assignees
No one assigned
Labels
examples Issue/PR related to examples
Projects
None yet
Milestone
No milestone
2 participants
@xhoantran @ijjk