i18n icon indicating copy to clipboard operation
i18n copied to clipboard

feat(config): add `site.url` as an alternative to `baseUrl`

Open dargmuesli opened this issue 1 year ago • 8 comments

🔗 Linked issue

https://github.com/nuxt-modules/i18n/issues/2474

❓ Type of change

  • [x] 📖 Documentation (updates to the documentation or readme)
  • [ ] 🐞 Bug fix (a non-breaking change that fixes an issue)
  • [ ] 👌 Enhancement (improving an existing functionality like performance)
  • [x] ✨ New feature (a non-breaking change that adds functionality)
  • [ ] ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

This PR adds support for reading Nuxt config's site.url, provided by Nuxt SEO's site-config module, marking i18n.baseUrl as deprecated.

Draft because:

  • [ ] nuxt-site-config does not (yet?) support options as functions, which has to be discussed

📝 Checklist

  • [x] I have linked an issue or discussion.
  • [x] I have updated the documentation accordingly.

dargmuesli avatar Oct 09 '23 01:10 dargmuesli

Live Preview ready!

Name Edit Preview Latest Commit
i18n Edit on Studio ↗︎ View Live Preview 79312457766921139e7588051d652dd41470cdf0

nuxt-studio[bot] avatar Oct 09 '23 01:10 nuxt-studio[bot]

While I'm in favor of supporting/using the config module, if we want to add this for v8 it will have to be backwards compatible. I feel it's important that we make an effort to prevent breaking changes between release candidates as we are working to stable release.

So hopefully there is a way to implement this while keeping compatibility and otherwise we will have to delay this until after v8 stable release.

BobbieGoede avatar Oct 10 '23 08:10 BobbieGoede

This change should be backwards compatible already and only make use of site.url if available. The only thing a user should see differ is the deprecation notice, which could theoretically be removed and reintroduced before v9. I haven't done extensive testing yet though as I first want to resolve the threads above I opened myself before doing testing.

dargmuesli avatar Oct 10 '23 15:10 dargmuesli

@BobbieGoede would you mind sharing what your understanding of the baseUrl does? I want to make sure we're covering the test cases and actually implementing the module makes sense.

From my understanding, it's just used for i18n head tags?

A couple of minor notes until I can do a proper review:

  • end user shouldn't have to migrate baseUrl to site.url, both should be valid - for now (we nudge towards site url for easier consolidation of config)
  • worth consolidating the trailingSlash config which nuxt-site-config also supports

harlan-zw avatar Oct 10 '23 16:10 harlan-zw

@dargmuesli

This change should be backwards compatible already and only make use of site.url if available. The only thing a user should see differ is the deprecation notice, which could theoretically be removed and reintroduced before v9.

Ah right, my bad. I mistook the deprecation notices as having the original baseURL handling removed for some reason 😅.

It's good to support setting the value using nuxt-site-config, but deprecating baseUrl will be something to decide later/separately. For simplicity (for users) I think it is important to keep the ability to configure the module in a single place (the i18n key or module array).

I haven't done extensive testing yet though as I first want to resolve the threads above I opened myself before doing testing.

One thing to consider is layer support, especially when supporting multiple ways of setting options. In the current implementation a layer will override a project's baseUrl if the layer uses this new approach.

@harlan-zw

From my understanding, it's just used for i18n head tags?

As I understand it, this is correct. Most of this happens in vue-i18n-routing, which uses the config to set up the head tags*.

It is possible to override baseUrl by setting domain on locales, or by using a function to set it based on the Nuxt Context as seen here. When setting domain on locales, it is possible to override these using runtime config as well. These options exist to allow projects to support different languages using different domains, I'm not sure how this would interact with nuxt-site-config or when other modules rely on it as well.

*On a separate note, I've been wanting to look into implementing/replacing the head tag handling in vue-i18n-routing with unhead. Unfortunately right now I'm not yet familiar enough with either of these packages to do this, if you have time to give it a look and/or point me in the right direction, that would be greatly appreciated! 😅

BobbieGoede avatar Oct 10 '23 18:10 BobbieGoede

Sorry, I was preparing for vue fes japan, so I could not track of what was going on with this issue. :sweat_smile: How is going this PR?

kazupon avatar Nov 13 '23 16:11 kazupon

It's on my TODO list, but as it should be a backwards compatible feature addition I had other tasks with higher priority. I think I can continue to work on it this week.

dargmuesli avatar Nov 14 '23 07:11 dargmuesli

I also need to re-review this properly when I have some time

harlan-zw avatar Nov 15 '23 04:11 harlan-zw