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

refactor!: read vue-i18n options from config path #1948

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/module.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { existsSync } from 'fs'
import createDebug from 'debug'
import { isBoolean, isObject, isString } from '@intlify/shared'
import { defineNuxtModule, isNuxt2, isNuxt3, getNuxtVersion, addPlugin, addTemplate, addImports } from '@nuxt/kit'
import { defineNuxtModule, isNuxt2, isNuxt3, getNuxtVersion, addPlugin, addTemplate, addImports, resolvePath } from '@nuxt/kit'
import { resolve, relative, isAbsolute } from 'pathe'
import { setupAlias, resolveVueI18nAlias } from './alias'
import { setupPages } from './pages'
Expand Down Expand Up @@ -95,6 +96,8 @@ export default defineNuxtModule<NuxtI18nOptions>({
? resolve(nuxt.options.rootDir, options.vueI18n)
: { legacy: false }

const configPath = await resolvePath(options.vueI18n?.configFile || 'vue-i18n.options', { cwd: nuxt.options.rootDir, extensions: ['.ts', '.mjs', '.js'] })

/**
* extend messages via 3rd party nuxt modules
*/
Expand Down
4 changes: 3 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ export type NuxtI18nOptions<Context = unknown> = {
skipSettingLocaleOnNavigate?: boolean
// sortRoutes?: boolean
strategy?: Strategies
vueI18n?: I18nOptions | string
vueI18n?: {
configFile?: string
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope the i18n options keep I18nOptions | string because it would break compatibility with v7.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would need to be string for this change. Sorry if I'm misunderstanding but why is compatibility with v7 important (since this is another major version)?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, v8 is a major version, so breaking changes are inevitable.
Upgrading to a major version for a large application that has already been built is a huge effort.

Some frameworks (e.g. larave, rails, and react) have been released so that major versions do not cause too much work for the user's application. They are releasing features supported in previous versions in working order, just as Vue3 supports the Options API.

I have once again thought about how much breaking change would detract from the migration experience for the user.

In this PR, it is a change in the configuration interface, so the affected code point is local.
Changing the vueI18n option would not be much of a problem.
Of course, we need to write a migration guide in the documentation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely - it would be great to have as less breaking changes as possible.

Similar to #1919 also moving some fragile configuration options to other solutions that Nuxt provides for stability, I'm aiming to do the same here. This may have been a small weak-point in v7, so having this solved in v8 would be great. The migration would indeed be documented.

types?: 'composition' | 'legacy'
debug?: boolean
dynamicRouteParams?: boolean
Expand Down