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: set locale domains via runtimeConfig (#2443) #2446

Merged

Conversation

cjpearson
Copy link
Contributor

πŸ”— Linked issue

#2443

❓ 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

This change allows setting locale domains via the runtime config. Currently when enabling differentDomains the app must be recompiled for each environment because the domains are set at compile time. With this change a single build can be used on different environments with different .env files.

e.g.

production .env

NUXT_PUBLIC_I18N_LOCALES_EN_DOMAIN=en.example.test
NUXT_PUBLIC_I18N_LOCALES_DE_DOMAIN=de.example.test

staging .env

NUXT_PUBLIC_I18N_LOCALES_EN_DOMAIN=en.staging.example.test
NUXT_PUBLIC_I18N_LOCALES_DE_DOMAIN=de.staging.example.test

πŸ“ Checklist

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

@nuxt-studio
Copy link

nuxt-studio bot commented Sep 26, 2023

βœ… Live Preview ready!

Name Edit Preview Latest Commit
i18n Edit on Studio β†—οΈŽ View Live Preview e3f2788

@cjpearson
Copy link
Contributor Author

Is this test maybe flaky? I can't reproduce it locally and it also seems to be causing the failures in #2442 and ee5e026

@BobbieGoede
Copy link
Collaborator

@cjpearson
Yes the tests are a bit flaky in general (tracked here #1998), so will be rerunning it. Fortunately it seems to only throw false failures and not the other way around πŸ˜…

@BobbieGoede
Copy link
Collaborator

Thanks for contributing!

I think this feature looks good, I prefer it over the approach currently described in the docs here: https://v8.i18n.nuxtjs.org/guide/different-domains#runtime-environment-variables

I think if you change/replace the example in the linked documentation to use this feature it would be perfect! What do you think @kazupon?

@cjpearson
Copy link
Contributor Author

Thanks! I made a pass at updating the guide with an example

@kazupon
Copy link
Collaborator

kazupon commented Sep 28, 2023

Thank you for your contribution!
I think this feature looks good too.
we would like to merge this feature, when you will ready for example and docs :)

@cjpearson
Copy link
Contributor Author

Hi @kazupon I updated the docs for runtime config and routing and added an example to the different domains guide. Is there anything else I should change in the docs?

@BobbieGoede
Copy link
Collaborator

I have raised the test job timeout, if you rebase your branch on next the tests should succeed (at least it won't fail because of the timeout)

@BobbieGoede
Copy link
Collaborator

Not sure why the tests keep failing repeatedly now, I know that it's not because of your code as they pass locally. Looks like the flaky test debt has caught up with us 😭, will take some time to look into this.

Copy link
Collaborator

kazupon commented Sep 28, 2023

Yeah.
The Other PRs such as #2441 are also flaky testing… we should fix that issue. 😿

@BobbieGoede BobbieGoede mentioned this pull request Sep 28, 2023
7 tasks
@BobbieGoede BobbieGoede mentioned this pull request Sep 30, 2023
7 tasks
@BobbieGoede BobbieGoede merged commit 2fdcb76 into nuxt-modules:next Oct 1, 2023
4 checks passed
DarthGigi pushed a commit to DarthGigi/i18n that referenced this pull request Apr 16, 2024
…modules#2446)

* feat: set locale domains via runtimeConfig (nuxt-modules#2443)

* docs: add example of runtime domain configuration to the guide
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants