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

Support for fallback / substituable language (nonExplicitSupportedLngs) #58

Closed
michalpulpan opened this issue Jan 2, 2023 · 9 comments

Comments

@michalpulpan
Copy link
Contributor

Hello!

First of all, thank you very much for maintaining this project.
I wanted to ask whether there's (or is planned) support for a feature nonExplicitSupportedLngs / fallbackLng of next-i18next that is quite common in NextJS world.
See: i18next/next-i18next#1930

I'm running e-commerce site on NextJS and it's necessary for me to store both locale and country in the URL (as, for example, de-AT or de-CH). So I might have two urls example.org/de-DE/produkt/1 and example.org/de-CH/produkt/1. Both routes share the same locale but they might present different prices ( vs. CHF).
The rewrite itself can be handled by middleware (I can share code later if anyone is interested, but it's fairly simple).

With this feature, next-i18next allows to define "substituable" locale de-CH --> de and de-AT --> de so folder public/locales doesn't have to look like:

.
└── public/
    └── locales/
        ├── de
        ├── de-CH
        └── de-AT

but

.
└── public/
    └── locales/
        ├── de

is sufficient.

It would be nice if this feature is also supported by next-translate-routes so that routes.json doesn't have to look like 😊 :

{
  "/": {
    "de": "produkt",
    "de-CH": "produkt",
    "de-AT": "produkt",
    }
}

but only

{
  "/": {
    "de": "produkt",
    }
}

and generate page only once (for de).

If it would not be possible to implement, is there a chance to at least to introduce substitutable pages into build process?
NextJS will now generate the same page 3x (de, de-CH, de-AT) and it heavily slows down build time (due to large amount of pages).

Thank you in advance ☺️ .

@cvolant
Copy link
Collaborator

cvolant commented Jan 20, 2023

I understand you need... but there is a lot of work to support next@13, and a new born baby plus a lot of work make it hard to find some time to work on it these days.
So unfortunately it is unlikely that I can work on this soon. You can try to make a PR though: there are some complexities, but I tried to make the code as clear as possible and well documented.

@michalpulpan
Copy link
Contributor Author

I understand you need... but there is a lot of work to support next@13, and a new born baby plus a lot of work make it hard to find some time to work on it these days.
So unfortunately it is unlikely that I can work on this soon. You can try to make a PR though: there are some complexities, but I tried to make the code as clear as possible and well documented.

Hello, I'm really sorry for being this late with my response, but I missed your reply.
I'll be honest, I haven't digged much into the code of next-translate-routes yet. But given your skill, you think it would be doable? Mainly as a solution for large amount of generated pages?

NextJS will now generate the same page 3x (de, de-CH, de-AT) and it heavily slows down build time (due to large amount of pages).

So it should only generate de version (for all these three variants) and solve possible issue with https://nextjs.org/docs/messages/max-custom-routes-reached? But, of course, maintain correct locale in the path, so for example:
example.com/de-CH/produkt/1, example.com/de-AT/produkt/1, etc.

Thank you for your response!

@cvolant
Copy link
Collaborator

cvolant commented Feb 17, 2023

I gave a quick look at it to estimate the work to be done. Let's say half a day of work if everything goes smoothly (it may double otherwise), including solving bugs, checking that everything works good, and writing tests.

There is at least 10 places in 5 files to update. If you want to see them, you can perform a search of "path.default" in next-translate-routes code... Each time, it is something like const path = routeBranch.paths[locale] || routeBranch.paths.default.
We would need to write a getPath function as follow then use it in this 10 places:

const getPath = <L extends string>({ paths, locale }: { paths: TRouteSegmentPaths<L>, locale: L }) => {
  if (paths[locale]) {
    return paths[locale]
  }
  const { fallbackLng } = getNtrData()
  if (fallbackLng?.[locale]) {
    for (const l of fallbackLng[locale]) {
      if (paths[l]) {
        return paths[l]
      }
    }
  }
  return paths.default
}

It might be enough... It may not. 🤞
The longer part will probably be to write tests and make sure everything works.
You can give it a try I you want.

@cvolant
Copy link
Collaborator

cvolant commented Feb 17, 2023

Note: the above don't includes nonExplicitSupportedLngs option that would need some more work.
I don't know how next-i18next handles it but for next-translate-routes it should be quite simple: building fallbackLng automatically on ntrData initialization should be enough.

@michalpulpan
Copy link
Contributor Author

Thank you very much for your valuable input and time. I'm currently a bit out of time so hopefully I'll get to this as soon as possible. This project is coming together very well!

@michalpulpan
Copy link
Contributor Author

michalpulpan commented Mar 8, 2023

Hello, @cvolant ,
I think I'm mostly done with the implementation of fallbackLng (and tests, hopefully it's sufficient, haha). We can work out nonExplicitSupportedLngs in separate PR.
I have branch feat/fallback-lng locally and sadly cannot push into it.

➜  next-translate-routes git:(feat/fallback-lng) git push origin feat/fallback-lng
ERROR: Permission to hozana/next-translate-routes.git denied to michalpulpan.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

Where my origin is:

➜  next-translate-routes git:(feat/fallback-lng) git remote -v
origin  git@github.com:hozana/next-translate-routes.git (fetch)
origin  git@github.com:hozana/next-translate-routes.git (push)

Am I doing something totally wrong or do I need permissions? 😊

Thanks!

@cvolant
Copy link
Collaborator

cvolant commented Mar 9, 2023

Hi @michalpulpan ,
Thank you for contributing!
Here is a short guide about how to submit a PR on an open-source project: https://opensource.com/article/19/7/create-pull-request-github
And here, a more in-depth, video guide, where the steps 7, 8, and 9 should help: https://egghead.io/courses/how-to-contribute-to-an-open-source-project-on-github

@michalpulpan
Copy link
Contributor Author

Hi @michalpulpan ,
Thank you for contributing!
Here is a short guide about how to submit a PR on an open-source project: https://opensource.com/article/19/7/create-pull-request-github
And here, a more in-depth, video guide, where the steps 7, 8, and 9 should help: https://egghead.io/courses/how-to-contribute-to-an-open-source-project-on-github

Hi,

Thank you very much! It's now absolutely clear 🤦. First time properly contributing to open-source 😄. I made draft of the PR #66. Looking forward to your feedback!

@cvolant
Copy link
Collaborator

cvolant commented Apr 28, 2023

Available in v1.10.0. Thank you @michalpulpan !

@cvolant cvolant closed this as completed Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants