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

Upgrade <Link> to reflect changes for dynamic routing #250

Closed
shunkakinoki opened this issue Sep 8, 2020 · 7 comments · Fixed by #255 or #285
Closed

Upgrade <Link> to reflect changes for dynamic routing #250

shunkakinoki opened this issue Sep 8, 2020 · 7 comments · Fixed by #255 or #285
Assignees
Milestone

Comments

@shunkakinoki
Copy link
Contributor

shunkakinoki commented Sep 8, 2020

Hi once again! Thanks for the awesome project

Since v9.5.3, it seems like the component was updated for dynamic routing,
making the prop as= behave as href=

Updating the canary package to reflect this would be great!

Links:
vercel/next.js#16774
vercel/next.js#16634

@aralroca
Copy link
Owner

aralroca commented Sep 9, 2020

Thanks to report it! This will be solved in the next release!

@aralroca aralroca self-assigned this Sep 9, 2020
@aralroca aralroca added this to the 0.17.3 milestone Sep 9, 2020
@aralroca
Copy link
Owner

aralroca commented Oct 2, 2020

With the PR I've done it solves at the moment all the dynamic routes except the case of using a custom server. The reason is that Next.js now forces an error when the href and as don't match. And for custom server it is necessary that there is the language as prefix in the as ...

An alternative to solve this would be to change the middleware to redirects.

Maybe I can merge the current PR leaving the custom server part for another PR later so that it doesn't block. What do you think about this @shunkakinoki ?

@shunkakinoki
Copy link
Contributor Author

@aralroca Thanks for the awesome work as always!!!
Definitely, a middleware approach would work for sure, to pass the PR. 🙇
Though with that being said, I'm not a user of the next.js custom server approach, so maybe the community will think otherwise.
Perhaps a canary version with warnings issued for custom server users for the time being?

@aralroca
Copy link
Owner

I reopen the issue because although I have merged the PR because it didn't block, we still need to solve the navigation with dynamic-routing + custom server

@aralroca aralroca reopened this Oct 10, 2020
@aralroca aralroca modified the milestones: 0.17.3, 0.18.0 Oct 10, 2020
@aralroca
Copy link
Owner

@shunkakinoki I prereleased the PR under 0.17.3-canary.4 if you want to try it.

@shunkakinoki
Copy link
Contributor Author

Thank you so much! You're the best

@aralroca
Copy link
Owner

From version 9.5.7 of Next.js, the Next.js Link component itself will have the same functionality as the next-translate Link.

I share the documentation on the subject:

https://github.com/vercel/next.js/pull/18067/files

This means that maybe we can deprecate the Link and Router from next-translate so that people can use the Link and Router from Next.js directly. You will only have to replace lang with locale.

It is still experimental, and we will have to make several tests before. So for the moment, I don't close this issue. But for me, it makes sense to evolve around here, since the same functionality that I implemented will now be in the Next.js core.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants