-
-
Notifications
You must be signed in to change notification settings - Fork 483
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: support layer locales and pages #1925
Conversation
This is great! Only thing that i have some doubts about is that the layers/extend functionality is a Nuxt default feature, and it actually does all the work before anything is loaded. Shouldn't nuxt-i18n work with the merged app, with the final result of the app and the extended layer, so that you shouldn't have any extra options for enabling or disabled it here. If someone is using the layers, and uses pages which are extended, i think that you can make the assumption that he want the files that is used, to be used the same way, as if it wasn't extended (if this makes any sense). |
Thank you for your PR! I have checked your logic, so it looks good to me. |
Love this! Would be great if @ memic84's gets addressed too π |
@memic84 & @ineshbose I agree that ideally it should fully work when extending, and I actually prefer having no option to disable it. The reason for adding those options is because I ran into issues trying to build a project when extending using layers from a remote source, with some bad troubleshooting I thought my changes were the cause of those issues, but it turns out this happens in a clean project as well. π I have opened an issue there nuxt/nuxt#19613. So the ability to disable these features will be removed unless we run into a real use case for it. Hope that resolves the doubts you described! While nuxt/nuxt#19613 does prevent building when extending using remote layers, it seems to work fine with local layers, I will make sure to mention it when adding the documentation. |
Yeah now it makes sense with the explanation, thanks! Let me know if i can help. I'll test it in my project asap. |
Should there be any logic to merging locales if they have domains? I'm probably overthinking it but here are some examples with different merging strategies: Scenario 1// project
// `differentDomains` is set to false
extends: ['layer1', 'layer2'],
i18n: {
differentDomains: false,
locales: [
{ code: 'en', file: 'en.json' }
{ code: 'fr', file: 'fr.json' }
],
} // layer 1
locales: [
{ code: 'en', file: 'en.json', domain: 'mydomain.com' }
]
// layer 2
locales: [
{ code: 'en', file: 'en.json', domain: 'otherdomain.com' }
] // result 1-1
// matching locale `code` have their files merged
locales: [
{ code: 'en', files: ['[layer2-path]/en.json', '[layer1-path]/en.json', 'en.json'] },
{ code: 'fr', files: ['fr.json'] }
]
// result 1-2
// locale `domain` do not match, do not merge
locales: [
{ code: 'en', files: ['en.json'] },
{ code: 'fr', files: ['fr.json'] }
]
// result 1-3
// merge locale options, ignoring differing options and merging based on `code`
// keeping in mind that earlier layers have higher priority (`domain` value is that of layer1)
locales: [
{ code: 'en', files: ['[layer2-path]/en.json', '[layer1-path]/en.json', 'en.json'], domain: 'mydomain.com', },
{ code: 'fr', files: ['fr.json'] }
] Scenario 2// project
// `differentDomains` is set to true, implying the consuming project cares about domain settings
extends: ['layer1', 'layer2'],
i18n: {
differentDomains: true,
locales: [
{ code: 'en', file: 'en.json', domain: 'mydomain.com' }
],
} // layer 1
locales: [
{ code: 'en', file: 'en.json', domain: 'mydomain.com' },
{ code: 'fr', file: 'fr.json', domain: 'mydomain.fr' }
]
// layer 2
locales: [
{ code: 'en', file: 'en.json', domain: 'otherdomain.com' },
{ code: 'fr', file: 'fr.json', domain: 'otherdomain.fr' }
] // result 2-1
// `domain` matching project are merged, first found locales are added, others are omitted
locales: [
{ code: 'en', files: ['[layer1-path]/en.json', 'en.json'], domain: 'mydomain.com' },
{ code: 'fr', files: ['[layer1-path]/fr.json'], domain: 'mydomain.fr' }
]
// result 2-2
// merge options regardless of domain, more translations are better
// keeping in mind that earlier layers have higher priority
locales: [
{ code: 'en', files: ['[layer2-path]/en.json', '[layer1-path]/en.json', 'en.json'], domain: 'mydomain.com' },
{ code: 'fr', files: ['[layer2-path]/fr.json', '[layer1-path]/fr.json'], domain: 'mydomain.fr' }
] |
11cd655
to
c1c8b7a
Compare
Thanks again for your contribution. I honestly, I don't know about locale merging for domain cases, because there is no best practice with layers yet nuxt official π
|
c1c8b7a
to
2615149
Compare
Is this going to be in the next release? |
@BobbieGoede |
I think we need to prepare the docs about nuxt layers |
@kazupon Sure! @memic84 @kazupon I was still working on an issue that shows up after building a project using this implementation, it seems like SSR doesn't have access to the merged locales. I will rebase this branch and add my latest changes today, perhaps you can see what the cause is faster than me π |
f3b5f78
to
afe8528
Compare
@kazupon I have resolved the conflicts and added a very basic test to demonstrate the current issue, the translation for the key At first I thought this might be related to the |
It looks like the way locale files were being handled in |
@kazupon I have added a page to the docs that briefly describes the functionality and added a few more tests. |
@BobbieGoede |
Thank you both!! I have one question however, see: https://v8.i18n.nuxtjs.org/guide/layers#pages-routing If you have pages in the layer, which is included in your project, that one doesn't seem to get merged together. I have both lazy and langDir in both configs. A reproduction: https://stackblitz.com/edit/nuxt-starter-5ar22s?file=nuxt.config.ts,base%2Fnuxt.config.ts,base%2Froutes%2FRoutes.js,package.json See also for more information: #1837 Could be that i am doing something wrong here. |
@memic84 the latest beta ( Let me know if it works after installing the |
@BobbieGoede I have tried the edge version, and this is what i get. Reproduction The same warning as before, that the route cannot be found:
Folder base is the extend folder, and there i have one page Do you have a playground or stackbilitz example where this works, so that i can see what i am doing wrong here? |
@memic84 Can you check if you're experiencing the same issue in this fork of your reproduction? https://stackblitz.com/edit/nuxt-starter-jrk2ex?file=nuxt.config.ts |
It works now in both versions. What did you change in the latest in next version? The problem seem to be the following, it seemed that for the routes which were used as children, such as
However i do get other issues in the edge versions, but those are not related to this pull request. Thank you for the solution! |
* feat: support layer locales and pages * fix: locales not merged when lazy is false * test: add basic layers test * fix: layer locales not being bundled correctly * docs: add layers page * feat: change playground to illustrate layer override behavior * test: add additional layer tests * refactor: restructure layers implementation * fix: update lockfile
π Linked issue
#1890
#1837
#1743
β Type of change
π Description
This PR implements the workaround as described in this comment. But I have added basic support for layer pages and routes as well.
Unfortunately it seems like there are issues with the current
extends
behavior when using a git repository as a layer. So when trying to use pages from a git repository layer, an error similar to nuxt/nuxt#18448 gets thrown. Locales from such layers do seem to work with this implementation.The resulting behavior when merging (lazy-loaded) locales
I have added basic tests and documentation, these can be expanded if needed.
π Checklist