nuxt-modules / sitemap

Powerfully flexible XML Sitemaps that integrate seamlessly, for Nuxt.
https://nuxtseo.com/sitemap
MIT License
336 stars 31 forks source link

feat: `nuxt-i18n-micro` module integration #357

Closed harlan-zw closed 2 months ago

harlan-zw commented 2 months ago

๐Ÿ”— Linked issue

https://github.com/s00d/nuxt-i18n-micro/issues/11

โ“ Type of change

๐Ÿ“š Description

As the i18n config has an identical signature we can support the https://github.com/s00d/nuxt-i18n-micro module out of the box without any extra effort :crossed_fingers:

vercel[bot] commented 2 months ago

The latest updates on your projects. Learn more about Vercel for Git โ†—๏ธŽ

Name Status Preview Updated (UTC)
sitemap-edge-demo โœ… Ready (Inspect) Visit Preview Sep 20, 2024 7:29am
genu commented 2 months ago

@harlan-zw is a similar change required here?

https://github.com/harlan-zw/nuxt-seo/blob/bc1dc0b4263adcf90d8df372c4b4eb21a937ba6e/src/module.ts#L143C5-L146C6

harlan-zw commented 2 months ago

Thanks for the review @genu, I'm only considering sitemap integration with this module at the moment.

genu commented 2 months ago

I see, but wouldn't it cause issue when using the main seo module?

For instance, it will polyfill the i18n composable when it should not, since its provided by nuxt-i18n-micro.

Nuxt throws errors about duplicate composables when using together.

harlan-zw commented 2 months ago

Hmm good point but it's a warning that is thrown so not a big deal, the pollyfills should be registered with low priority though.

Having to support nuxt-i18n-micro in all SEO modules is not something I have capacity for at the moment so if this is a blocker then we may consider closing this PR.

genu commented 2 months ago

Hmm, I can test a little bit more on my end, as I noticed the polyfill is indeed used instead of the i18n-micro composable.

Maybe the order in the modules array affects this? ๐Ÿค”