-
Notifications
You must be signed in to change notification settings - Fork 0
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
Creates i18n infrastructure #40
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, main general comment is that not all links have been updated [1], so you occassionally just get dumped back to the default language.
Is there a scheme (within vue/nuxt-i18n) we can use that's a little more bulletproof? It's easy to forget localePath
when adding links/buttons/general nav
[1] /pages/admin/index.vue
and /pages/admin/pacta-version/index.vue
were the two I noticed
@@ -19,6 +19,7 @@ interface PACTAError extends NuxtError { | |||
} | |||
|
|||
export const createErrorWithRemediation = (err: string | Partial<NuxtError>, r: Remediation): PACTAError => { | |||
console.log(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove? This should get logged up the stack if it's thrown correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These aren't showing up in logs, and it's proving challenging to debug stack traces without them.
frontend/nuxt.config.ts
Outdated
@@ -41,4 +44,24 @@ export default defineNuxtConfig({ | |||
], | |||
dirs: ['globalimports'], | |||
}, | |||
i18n: { | |||
// TODO(grady) Set a Base URL once we have it for SEO https://i18n.nuxtjs.org/guide/seo#:~:text=You%20must%20also%20set%20the%20baseUrl%20option%20to%20your%20production%20domain%20in%20order%20to%20make%20alternate%20URLs%20fully%2Dqualified%3A | |||
// baseUrl: 'https://my-nuxt-app.com' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we do have a base URL! In dev, it's https://pacta.dev.rmi.siliconally.dev
. Seems like this should be populated from envs/
like other env-specific config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GC fixed
Great question + suggestion. I ended up creating some middleware that tests for this, and logs in the console when we fail to meet it. We can beef that up if/when we need to. That (in tandem with making the prefix consistent) will make these navigation errors a lot easier to spot and remediate, though I wish we could do something more compiley. |
vue-i18n
andnuxt-i18n
, configured such that:/fr/admin
) etc. This is easy to change if RMI wants to later.accept-lang
and a few other heuristics, but honor a user's choice once made./admin
and/en/admin
go to the same place)VueI18nVitePlugin
- you can see in the network tab when we switch langauges we fetch thelang/*.json?import
file.objectNotation: true
to allowPrefix.Suffix
to be a nested property in JSON.prefix
- seems like an appropriate thing for now.prefix
witht
in the template to create a helpertt
to avoid having to write prefixes.