-
-
Notifications
You must be signed in to change notification settings - Fork 8
Add dynamic route translation and language switching for paginated routes #46
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
base: main
Are you sure you want to change the base?
Conversation
…utes
Summary
This PR fixes two critical issues with dynamic route handling and language switching:
1. Dynamic route translation: Fixed parent route pattern matching for routes like /news/[...page] that weren’t being properly translated to localized paths like /fr/actualites/[...page].
2. Language switching preservation: Fixed switchLocalePath to preserve current page parameters when switching languages on paginated routes.
Issues Fixed
- Routes like /news/[...page] now correctly translate to /fr/actualites/[...page] instead of /fr/news/[...page].
- Language switching from /fr/actualites/2 now correctly goes to /it/notizie/2 instead of redirecting to home page.
- Handles complex route names with hyphens correctly (e.g., /news-copy → /actualites-copy → /notizie-copy).
- Works for both paginated routes (preserves page numbers) and non-paginated routes (preserves exact paths).
Technical Details
File: package/src/routing/register.ts
- Improved parent pattern matching logic in generateRoute function.
- Fixed edge case where segments.slice(0, i).join('/') || '/' was incorrectly falling back to root path.
File: package/assets/stubs/virtual.mjs
- Enhanced switchLocalePath function to extract current page parameters from URL.
- Added distinction between exact path matches and paginated route matches.
- Fixed parameter extraction for rest routes to prevent incorrect URL segments like /notizie/-copy.
Demo Updates and Playground
To facilitate testing, the demo has been enhanced with:
- Added new blog posts in all locales (second-post.md, deuxieme-article.md, secondo-articolo.md).
- Created /news/[...page] route with pagination functionality.
- Added route translations for /news → /actualites (FR) / /notizie (IT).
- Configured pagination with pageSize: 1 to easily test multiple pages.
- Added navigation links in header for easy testing.
Test Cases
- /fr/actualites/2 → switch to IT → /it/notizie/2.
- /fr/actualites-copy/3 → switch to IT → /it/notizie-copy/3.
- /fr/actualites-copy → switch to IT → /it/notizie-copy.
- Static routes continue to work as expected.
✅ Deploy Preview for astro-i18n ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for astro-i18n-demo ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
#2 This issue has now been resolved: the library supports rest parameter translations as well |
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.
.
Summary - Fixed content collection schema errors by adding postsGlob collection with proper references - mproved parameter mapping in switchLocalePath for better route translation - Updated news pagination to use new glob-based collection Test plan - Verify dev server starts without content collection errors - Test news pagination pages load correctly - Verify language switching works on paginated routes
|
@florian-lefebvre by the way, I see that you are part of the core Astro team. |
|
Hi, thanks a lot for sending a PR! To be honest, given I do not have the capacity to maintain this package, I'm not super confident merging this because we don't have tests so I wouldn't want to break existing projects. I'll share it in the
We talked about it in withastro/roadmap#1176 but there's no consensus yet, I'd love to see it tackled but realistically, that won't happen before a while (if it happens) |
|
@florian-lefebvre Yes, it would be great if someone else could test it as well. I’m already working on my own starter and have tested it on my side. I also found Advanced-Astro-i18n, which uses this library, and the recent update is already working on that project. If someone else can take a look, even better! Tnx |
|
I'll take the time next week 👍, can you run the formatter? I think that would reduce the diff a little bit |
|
Great. Thank you Florian! |
florian-lefebvre
left a comment
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.
I haven't touched this code in a long time so I trust you! You said you tested it on the i18n starter, the demo works so I think it's fine. Just some little questions
| }); | ||
| }) satisfies GetStaticPaths; | ||
| const { page } = Astro.props as { page: Page; authors: any[] }; |
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.
Do you need the type assertion here?
| }), | ||
| }); | ||
|
|
||
| export const collections = { |
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 not clear to me what's the difference between the 2 collections
Summary
This PR addresses critical content collection schema errors and improves i18n routing functionality:
getLocalePath("/news")to work when only child routes like/news/[...page]existIssues Fixed
getLocalePath("/news")now finds child routes like/news/[...page]and returns the correct translated base pathswitchLocalePathno longer crashes with "Must provide [param] param" errors in head componentsTechnical Details
File:
demo/src/content/config.tssrc/data/postsdirectoryFile:
demo/astro.config.tsFile:
demo/src/routes/news/[...page].astroFile:
package/assets/stubs/virtual.mjsgetLocalePathwith parent route discovery for cases where only child routes existswitchLocalePathparameter validation to gracefully handle missing parametersContent Structure
Enhanced content organization with:
src/data/postsUse Cases
These changes enable several important scenarios:
getLocalePath("/blog")works even if only/blog/[slug]routes existgetSwitcherData()works reliably in<head>context without parameter errors