-
Notifications
You must be signed in to change notification settings - Fork 393
[feat] implement dynamic imports for locale code splitting #6076
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
Changes from 2 commits
85ec556
b2168d1
1274a07
8611d94
07f1406
41b15a4
c63ed4e
8f802d7
2b56274
34364ca
a57792b
8d86d41
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,45 +1,10 @@ | ||
| import { createI18n } from 'vue-i18n' | ||
|
|
||
| import arCommands from './locales/ar/commands.json' with { type: 'json' } | ||
| import ar from './locales/ar/main.json' with { type: 'json' } | ||
| import arNodes from './locales/ar/nodeDefs.json' with { type: 'json' } | ||
| import arSettings from './locales/ar/settings.json' with { type: 'json' } | ||
| // Import only English locale eagerly as the default/fallback | ||
| import enCommands from './locales/en/commands.json' with { type: 'json' } | ||
| import en from './locales/en/main.json' with { type: 'json' } | ||
| import enNodes from './locales/en/nodeDefs.json' with { type: 'json' } | ||
| import enSettings from './locales/en/settings.json' with { type: 'json' } | ||
| import esCommands from './locales/es/commands.json' with { type: 'json' } | ||
| import es from './locales/es/main.json' with { type: 'json' } | ||
| import esNodes from './locales/es/nodeDefs.json' with { type: 'json' } | ||
| import esSettings from './locales/es/settings.json' with { type: 'json' } | ||
| import frCommands from './locales/fr/commands.json' with { type: 'json' } | ||
| import fr from './locales/fr/main.json' with { type: 'json' } | ||
| import frNodes from './locales/fr/nodeDefs.json' with { type: 'json' } | ||
| import frSettings from './locales/fr/settings.json' with { type: 'json' } | ||
| import jaCommands from './locales/ja/commands.json' with { type: 'json' } | ||
| import ja from './locales/ja/main.json' with { type: 'json' } | ||
| import jaNodes from './locales/ja/nodeDefs.json' with { type: 'json' } | ||
| import jaSettings from './locales/ja/settings.json' with { type: 'json' } | ||
| import koCommands from './locales/ko/commands.json' with { type: 'json' } | ||
| import ko from './locales/ko/main.json' with { type: 'json' } | ||
| import koNodes from './locales/ko/nodeDefs.json' with { type: 'json' } | ||
| import koSettings from './locales/ko/settings.json' with { type: 'json' } | ||
| import ruCommands from './locales/ru/commands.json' with { type: 'json' } | ||
| import ru from './locales/ru/main.json' with { type: 'json' } | ||
| import ruNodes from './locales/ru/nodeDefs.json' with { type: 'json' } | ||
| import ruSettings from './locales/ru/settings.json' with { type: 'json' } | ||
| import trCommands from './locales/tr/commands.json' with { type: 'json' } | ||
| import tr from './locales/tr/main.json' with { type: 'json' } | ||
| import trNodes from './locales/tr/nodeDefs.json' with { type: 'json' } | ||
| import trSettings from './locales/tr/settings.json' with { type: 'json' } | ||
| import zhTWCommands from './locales/zh-TW/commands.json' with { type: 'json' } | ||
| import zhTW from './locales/zh-TW/main.json' with { type: 'json' } | ||
| import zhTWNodes from './locales/zh-TW/nodeDefs.json' with { type: 'json' } | ||
| import zhTWSettings from './locales/zh-TW/settings.json' with { type: 'json' } | ||
| import zhCommands from './locales/zh/commands.json' with { type: 'json' } | ||
| import zh from './locales/zh/main.json' with { type: 'json' } | ||
| import zhNodes from './locales/zh/nodeDefs.json' with { type: 'json' } | ||
| import zhSettings from './locales/zh/settings.json' with { type: 'json' } | ||
|
|
||
| function buildLocale<M, N, C, S>(main: M, nodes: N, commands: C, settings: S) { | ||
| return { | ||
|
|
@@ -50,17 +15,114 @@ function buildLocale<M, N, C, S>(main: M, nodes: N, commands: C, settings: S) { | |
| } | ||
| } | ||
|
|
||
| // Locale loader map - dynamically import locales only when needed | ||
| const localeLoaders: Record< | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [architecture] low Priority Issue: Significant code duplication between src/i18n.ts and apps/desktop-ui/src/i18n.ts |
||
| string, | ||
| () => Promise<{ default: Record<string, unknown> }> | ||
| > = { | ||
| ar: () => import('./locales/ar/main.json'), | ||
| es: () => import('./locales/es/main.json'), | ||
| fr: () => import('./locales/fr/main.json'), | ||
| ja: () => import('./locales/ja/main.json'), | ||
| ko: () => import('./locales/ko/main.json'), | ||
| ru: () => import('./locales/ru/main.json'), | ||
| tr: () => import('./locales/tr/main.json'), | ||
| zh: () => import('./locales/zh/main.json'), | ||
| 'zh-TW': () => import('./locales/zh-TW/main.json') | ||
| } | ||
|
|
||
| const nodeDefsLoaders: Record< | ||
| string, | ||
| () => Promise<{ default: Record<string, unknown> }> | ||
| > = { | ||
| ar: () => import('./locales/ar/nodeDefs.json'), | ||
| es: () => import('./locales/es/nodeDefs.json'), | ||
| fr: () => import('./locales/fr/nodeDefs.json'), | ||
| ja: () => import('./locales/ja/nodeDefs.json'), | ||
| ko: () => import('./locales/ko/nodeDefs.json'), | ||
| ru: () => import('./locales/ru/nodeDefs.json'), | ||
| tr: () => import('./locales/tr/nodeDefs.json'), | ||
| zh: () => import('./locales/zh/nodeDefs.json'), | ||
| 'zh-TW': () => import('./locales/zh-TW/nodeDefs.json') | ||
| } | ||
|
|
||
| const commandsLoaders: Record< | ||
| string, | ||
| () => Promise<{ default: Record<string, unknown> }> | ||
| > = { | ||
| ar: () => import('./locales/ar/commands.json'), | ||
| es: () => import('./locales/es/commands.json'), | ||
| fr: () => import('./locales/fr/commands.json'), | ||
| ja: () => import('./locales/ja/commands.json'), | ||
| ko: () => import('./locales/ko/commands.json'), | ||
| ru: () => import('./locales/ru/commands.json'), | ||
| tr: () => import('./locales/tr/commands.json'), | ||
| zh: () => import('./locales/zh/commands.json'), | ||
| 'zh-TW': () => import('./locales/zh-TW/commands.json') | ||
| } | ||
|
|
||
| const settingsLoaders: Record< | ||
| string, | ||
| () => Promise<{ default: Record<string, unknown> }> | ||
| > = { | ||
| ar: () => import('./locales/ar/settings.json'), | ||
| es: () => import('./locales/es/settings.json'), | ||
| fr: () => import('./locales/fr/settings.json'), | ||
| ja: () => import('./locales/ja/settings.json'), | ||
| ko: () => import('./locales/ko/settings.json'), | ||
| ru: () => import('./locales/ru/settings.json'), | ||
| tr: () => import('./locales/tr/settings.json'), | ||
| zh: () => import('./locales/zh/settings.json'), | ||
| 'zh-TW': () => import('./locales/zh-TW/settings.json') | ||
| } | ||
|
|
||
| // Track which locales have been loaded | ||
| const loadedLocales = new Set<string>(['en']) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [performance] low Priority Issue: loadedLocales Set grows indefinitely without cleanup mechanism |
||
|
|
||
| /** | ||
| * Dynamically load a locale and its associated files (nodeDefs, commands, settings) | ||
| */ | ||
| export async function loadLocale(locale: string): Promise<void> { | ||
| if (loadedLocales.has(locale)) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [quality] medium Priority Issue: Potential race condition in concurrent loadLocale calls |
||
| return | ||
| } | ||
|
|
||
| const loader = localeLoaders[locale] | ||
| const nodeDefsLoader = nodeDefsLoaders[locale] | ||
| const commandsLoader = commandsLoaders[locale] | ||
| const settingsLoader = settingsLoaders[locale] | ||
|
|
||
| if (!loader || !nodeDefsLoader || !commandsLoader || !settingsLoader) { | ||
| console.warn(`Locale "${locale}" is not supported`) | ||
| return | ||
| } | ||
|
|
||
| try { | ||
| const [main, nodes, commands, settings] = await Promise.all([ | ||
| loader(), | ||
| nodeDefsLoader(), | ||
| commandsLoader(), | ||
| settingsLoader() | ||
| ]) | ||
|
|
||
| const messages = buildLocale( | ||
| main.default, | ||
| nodes.default, | ||
| commands.default, | ||
| settings.default | ||
| ) | ||
|
|
||
| i18n.global.setLocaleMessage(locale, messages as any) | ||
| loadedLocales.add(locale) | ||
| } catch (error) { | ||
| console.error(`Failed to load locale "${locale}":`, error) | ||
| throw error | ||
| } | ||
| } | ||
|
|
||
| // Only include English in the initial bundle | ||
| const messages = { | ||
| en: buildLocale(en, enNodes, enCommands, enSettings), | ||
| zh: buildLocale(zh, zhNodes, zhCommands, zhSettings), | ||
| 'zh-TW': buildLocale(zhTW, zhTWNodes, zhTWCommands, zhTWSettings), | ||
| ru: buildLocale(ru, ruNodes, ruCommands, ruSettings), | ||
| ja: buildLocale(ja, jaNodes, jaCommands, jaSettings), | ||
| ko: buildLocale(ko, koNodes, koCommands, koSettings), | ||
| fr: buildLocale(fr, frNodes, frCommands, frSettings), | ||
| es: buildLocale(es, esNodes, esCommands, esSettings), | ||
| ar: buildLocale(ar, arNodes, arCommands, arSettings), | ||
| tr: buildLocale(tr, trNodes, trCommands, trSettings) | ||
| en: buildLocale(en, enNodes, enCommands, enSettings) | ||
| } | ||
|
|
||
| export const i18n = createI18n({ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,7 +45,7 @@ import { useCoreCommands } from '@/composables/useCoreCommands' | |
| import { useErrorHandling } from '@/composables/useErrorHandling' | ||
| import { useProgressFavicon } from '@/composables/useProgressFavicon' | ||
| import { SERVER_CONFIG_ITEMS } from '@/constants/serverConfig' | ||
| import { i18n } from '@/i18n' | ||
| import { i18n, loadLocale } from '@/i18n' | ||
| import { useSettingStore } from '@/platform/settings/settingStore' | ||
| import { useFrontendVersionMismatchWarning } from '@/platform/updates/common/useFrontendVersionMismatchWarning' | ||
| import { useVersionCompatibilityStore } from '@/platform/updates/common/versionCompatibilityStore' | ||
|
|
@@ -151,10 +151,12 @@ watchEffect(() => { | |
| ) | ||
| }) | ||
|
|
||
| watchEffect(() => { | ||
| watchEffect(async () => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [architecture] low Priority Issue: async watchEffect could cause unexpected behavior if locale changes rapidly |
||
| const locale = settingStore.get('Comfy.Locale') | ||
| if (locale) { | ||
| i18n.global.locale.value = locale as 'en' | 'zh' | 'ru' | 'ja' | ||
| // Load the locale dynamically if not already loaded | ||
| await loadLocale(locale) | ||
|
||
| ;(i18n.global.locale as any).value = locale | ||
|
||
| } | ||
| }) | ||
|
|
||
|
|
||
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.
[architecture] high Priority
Issue: loadLocale function is not exported in desktop-ui version but is used
Context: The function exists but is not exported, preventing external usage for locale switching
Suggestion: Add 'export' keyword before the loadLocale function declaration on this line
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.
Fixed in 41b15a4 - exported the loadLocale function to make it available for external usage.