-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[example] Make sure next.js Links can accept url objects as href #19073
Conversation
just like next.js links can.
No bundle size changes comparing e170593...d0c4bc6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's documented in https://nextjs.org/docs/api-reference/next/link#with-url-object.
@Janpot Thanks :). I had a look at the table of content link issue, it doesn't seem that Next.js will prioritize a fix. If you want to apply the proposed patch in the issue you have open, I think that we could move forward with it. |
@oliviertassinari I'm not 100% sure about the regarding the hash links, I remember trying out the proposed fix and getting strange behavior, especially around trailing slashes. It seems like |
I think that it should ignore query parameters.
The biggest unknown is the SEO impact. |
@oliviertassinari As long as pages with and without trailing slash either 301 redirect to the same url or have the same canonical url, the SEO impact should be none/minimal. In the meantime I opened vercel/next.js#9973 |
Unless Google has some specific logic for handling trailing vs no trailing, introducting a 301 would impact ranking for a couple of months. We have already experienced it with v3 to v4 pathname renames. |
I'd be surprised if they didn't already handle that specific case, but I don't want to introduce new redirects, the current situation 301 redirects non-trailing to trailing and that's good. |
OK, maybe we will be able to remove |
next.js links can accept parsed url objects, this PR makes sure the material ui wrapper in the example can accept these as well.