nuxt-modules / i18n

I18n module for Nuxt
https://i18n.nuxtjs.org
MIT License
1.76k stars 485 forks source link

Custom route config doesn't cascade #3150

Open Tofandel opened 1 month ago

Tofandel commented 1 month ago

Describe the feature

Assuming the following page structure

 order
 |  index.vue
 |  [id].vue

I would assume the following i18n config

{
    strategy: 'prefix',
    locales: [
      { code: 'en', label: 'English', language: 'en-US', file: 'en.json' },
      { code: 'hu', label: 'Magyar', language: 'hu-HU', file: 'hu.json' },
    ],
    defaultLocale: 'en',
    customRoutes: 'config', // disable custom route with page components
    pages: {
      order: {
        en: '/order',
        hu: '/rendeles'
      },
    },
}

To result in the following paths

/en/order
/en/order/[id]
/hu/rendeles
/hu/rendeles/[id]

Instead currently no modification is done, matching has to be done on the vue file name and match perfectly

Resulting in

/en/order
/en/order/[id]
/hu/order
/hu/order/[id]

To get the desired behavior above we instead need to write this

pages: {
  'order/index': { // the index part seems like a bug TBH
    en: '/order',
    hu: '/rendeles'
  },
  'order/[id]': {
    en: '/order/[id]',
    hu: '/rendeles[id]'
  },
},

This is very verbose, duplicates a lot of translated words and not practical when there is a lot of subroutes and attributes and we want them all to have the base translated, because we need to write one entry for every file that is created

Could we instead resolve routes in a cascading fashion, so if the config matches the start of the path we replace the matching part with what was configured? (Behind an option maybe)

Additional information

Final checks

Tofandel commented 1 month ago

I see a few options for this:

  1. Add a keyword directly in the page definition:
pages: {
   order: {
       cascade: true,
       hu: '/rendeles',
   }
}
  1. Add slash in front of the page name to indicate it's a path and needs cascading, with an optional wildcard in front to indicate that path is a part only and could be after anything
pages: {
   '/order': {
       hu: '/rendeles',
   },
   '*/page': {
      hu: '/oldalon'
   }
}
  1. Add a wildcard character in the page definition that would have the same effect as the previous option
pages: {
   'order/*': {
       hu: '/rendeles/*',
   },
   '*/page/*': {
      hu: '*/oldalon/*'
   }
}
  1. Another option than pages lets call it segments, that would allow to replace whole words in the final urls once the pages resolver has run (skipping over parameters obviously)

Like this:

segments: {
  'order': {
    hu: 'rendeles',
  }
}

The last option has the advantage of giving the possibility to translate path in the middle of params which the first doesn't

Here is a real world example

Before:

pages: {
      'shop/[...category]/page/[page]': {
        en: '/product-category/[...category]/page/[page]',
        hu: '/termek-kategoria/[...category]/oldalon/[page]',
      },
}

After with option 2:

pages: {
      '/shop': {
        en: '/product-category',
        hu: '/termek-kategoria',
      },
      '*/page': {
        en: '/page',
        hu: '/oldalon',
      }
}

After with option 4:

segments: {
      'shop': {
        en: 'product-category',
        hu: 'termek-kategoria',
      },
      'page': {
        en: 'page',
        hu: 'oldalon',
      }
}

Looking for feedback on the prefered implementation for a PR

BobbieGoede commented 1 month ago

I can see this getting complicated when multiple layers provide such configs, matching pages isn't straightforward when taking redirects and aliases into account (not sure if these are supported in the current implementation though).

While your feature proposal is much appreciated 🙏 I'm not really convinced this functionality improves the dev experience enough to justify the maintenance and (potential) edge cases down the line. If your project has enough routes for this to be an issue I think writing a function to generate the config should make things easier 🤔

Tofandel commented 1 month ago

If your project has enough routes for this to be an issue I think writing a function to generate the config should make things easier

That would be the case for anyone using this lib TBH, because if you have a folder and say 4-5 pages in it, any translation of the root folder needs to be done for each component, so 5 times, and now imagine how it is when you have nested subfolders, it quickly becomes impossible to translate a folder path without writing either a hook to replace the name on all routes or a fs reader to generate the config.. This is really not the dev's place to do this as well, this is where the lib should step in and provide said function to generate the config

The current implementation is simply not usefull and cannot be used for more advanced projects in it's current state

Localization is meant to be done by less technical people and we need to be able to give them a way to easily translate paths without having to take care of all the parameters and duplication

matching pages isn't straightforward when taking redirects and aliases into account (not sure if these are supported in the current implementation though)

They are not, a redirect needs to be specified like this currently

routeRoules: {
  '/hu/shop/**': {
    redirect: {
      to: '/hu/termek-kategoria/**',
      statusCode: 301,
    },
  },
  '/en/shop/**': {
    redirect: {
      to: '/en/product-category/**',
      statusCode: 301,
    },
  }
}

I don't see how this would create more edge cases and maintenance than the current implementation, because it provides literally the same feature, that is to change the path of a route depending on the locale, just with a better config api

I already looked into implementing option 4 and it was pretty straightforward

I can see this getting complicated when multiple layers provide such configs

I also imagine this could be complicated with layers, but why would anyone provide this config in the layer? This is translation and is only meant for the end config and this argument could also be used for the current implementation, this has always been a problem for any array/object config option and it has solutions: https://github.com/nuxt/nuxt/issues/22196