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

feat: add basic module locale merging #1955

Merged
merged 5 commits into from
Mar 29, 2023

Conversation

BobbieGoede
Copy link
Collaborator

@BobbieGoede BobbieGoede commented Mar 27, 2023

πŸ”— Linked issue

#1942

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Allows modules to provide locales in a similar way to layers (#1925), as a potential replacement for i18n:extend-messages hook.

Current usage is using a hook to register your module with its i18n configuration, similar to configuring lazy-load locales. Other locale configuration have not been implemented yet. If a different approach is preferred please let me know!

export default defineNuxtModule({
  async setup(options, nuxt) {
    const { resolve } = createResolver(import.meta.url)
    nuxt.hook('i18n:registerModule', registerI18nModule => {
      registerI18nModule({
        langDir: resolve('./locales'),
        locales: [
          {
            code: 'en',
            iso: 'en-US',
            file: 'en.json',
            name: 'English'
          },
          {
            code: 'fr',
            iso: 'fr-FR',
            file: 'fr.json',
            name: 'Francais'
          },
          {
            code: 'nl',
            iso: 'nl-NL',
            file: 'nl.json',
            name: 'Nederlands'
          }
        ]
      })
    })
  }
})

This PR is still a work in progress, implementation will be more generic to prevent code duplication as it has in its current state. I will add tests and docs later if it's clear that we want this functionality to be added and the usage is agreed on.

πŸ“ Checklist

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

Copy link
Collaborator

kazupon commented Mar 28, 2023

This is great PR! ❀️

We can replace the i18n:extend-messages hook.
The problem of i18n:extend-messages hook is that at nuxt build-time, object locale is pre-compiled to javascript, so that vue-i18n can handle it. And then pass it to template (i18n options.mjs) in gen.ts.
This was javascript to javascript and unsafe. And if the locale object was huge, i18n options.mjs was huge too.

This PR solves these problems by lazy loading of the locale on runtime builds and Nuxt layers.
We can deprecated the i18n:extend-messages hook.

@BobbieGoede BobbieGoede marked this pull request as ready for review March 29, 2023 12:19
@BobbieGoede
Copy link
Collaborator Author

@kazupon I added some documentation as well as a basic test and I haven't touched the i18n:extend-messages hook to keep this PR from breaking existing functionality, so deprecation can be done in a separate PR.

The i18n:registerModule hook implementation only supports adding lazy-load translations (locales: LocaleObject[]), I feel that adding support for strings only (locales: ['en', 'fr']) wouldn't make sense as no messages would be added but I can add it if needed.

Copy link
Collaborator

kazupon commented Mar 29, 2023

@BobbieGoede
Thank you very much!
Great job!

I haven't touched the i18n:extend-messages hook to keep this PR from breaking existing functionality, so deprecation can be done in a separate PR.

I will write the documentation for i18n:extend-messages deprecation/

The i18n:registerModule hook implementation only supports adding lazy-load translations (locales: LocaleObject[]), I feel that adding support for strings only (locales: ['en', 'fr']) wouldn't make sense as no messages would be added but I can add it if needed.

Yes! locales: LocaleObject[] only is enough! locales: string[] makes no sense at all.

@kazupon kazupon merged commit d898a07 into nuxt-modules:next Mar 29, 2023
8 checks passed
@kazupon kazupon removed the WIP label Mar 29, 2023
DarthGigi pushed a commit to DarthGigi/i18n that referenced this pull request Apr 16, 2024
* feat: add basic module locale merging

* refactor: rewrite LocaleObject merging to be generic and reusable

* fix: register module hook type

* docs: add documentation describing registerModule hook

* test: add register module hook test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants