Migrate translations from Transifex to i18next#37
Conversation
Replace @transifex/native and @transifex/react with i18next and react-i18next. Use custom RxDBBackend that fetches translations from jsDelivr CDN and caches them in RxDB. Translation version is sourced from site.wcpos_version via useObservableEagerState so CDN URLs point to the correct release tag. Remove _tags from all t() calls across 166 files since namespace is now set via defaultNS config. Delete cache.ts files that are superseded by the backend plugins.
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughReplaces Transifex-based i18n with an i18next/react-i18next provider and custom RxDBBackend, removes a custom translation cache, updates package deps, performs a bulk removal of Changes
Sequence Diagram(s)sequenceDiagram
participant App as App (React)
participant Prov as TranslationProvider (i18next)
participant DB as translationsState (RxDB)
participant CDN as jsDelivr (CDN)
App->>Prov: mount / render provider
Prov->>DB: read(locale, wcposVersion)
DB-->>Prov: return cached translations (if any)
Prov->>i18next: createInstance/init + addResourceBundle(cached)
Prov->>CDN: async fetch translations for locale/version
CDN-->>Prov: respond with translations (if newer)
Prov->>DB: persist updated translations
Prov->>i18next: addResourceBundle(updated)
App->>i18next: useTranslation() → t(...) calls resolve strings
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
🚀 Deployment Summary
🔗 Quick Links🤖 Updated by GitHub Actions |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/screens/main/pos/cart/buttons/pay.tsx (1)
58-65:⚠️ Potential issue | 🟡 MinorPreserve error details when non-Error values are thrown.
If
erroris a string or plain object,error.messageis undefined and the user-facing message becomes the generic fallback. Consider using the same fallback logic you already use in the context so the toast can surface the actual error text.✅ Suggested fix
- orderLogger.error(t('{message}', {message: error.message || 'Error' }), { + const errorMessage = error instanceof Error ? error.message : String(error); + orderLogger.error(t('{message}', { message: errorMessage || 'Error' }), { showToast: true, saveToDb: true, context: { errorCode: ERROR_CODES.TRANSACTION_FAILED, - error: error instanceof Error ? error.message : String(error), + error: errorMessage, }, });packages/core/src/screens/main/customers/add.tsx (1)
58-66:⚠️ Potential issue | 🟡 MinorExtract error message once and reuse across both log message and context object.
Line 59 accesses
error.messagedirectly without type narrowing, while line 64 properly guards withinstanceof Errorcheck. This inconsistency can be resolved by deriving the safe string once at the beginning of the catch block and reusing it in both places.Suggested fix
} catch (error) { - mutationLogger.error(t('{message}', {message: error.message || 'Error' }), { + const errorMessage = + error instanceof Error ? error.message : String(error ?? 'Error'); + mutationLogger.error(t('{message}', { message: errorMessage }), { showToast: true, saveToDb: true, context: { errorCode: ERROR_CODES.TRANSACTION_FAILED, - error: error instanceof Error ? error.message : String(error), + error: errorMessage, }, });
🤖 Fix all issues with AI agents
In `@packages/core/src/contexts/translations/index.tsx`:
- Around line 22-25: The i18nInstance is memoized only on translationsState so
RxDBBackend.init() captures a stale wcposVersion; update the useMemo dependency
array for i18nInstance to include wcposVersion so a new i18nInstance (and thus
RxDBBackend.init()) runs when the version changes, ensuring the backend's
captured version used by read()/reloadResources() is current; locate the useMemo
that creates i18nInstance and add wcposVersion to its dependency list.
In `@packages/core/src/screens/auth/components/wp-user.tsx`:
- Around line 128-133: The user-facing copy in AlertDialogDescription inside
wp-user.tsx uses "effect" instead of "affect"; update the string passed to the
t(...) call to read "Are you sure you want to remove this user? Removing a user
from the POS will not affect any data on the server." and, if your i18n system
uses source strings as keys, update the corresponding translation entries for
that key (or add a new key) so translations remain correct; reference the
AlertDialogDescription component and the t(...) invocation to locate and change
the text.
In `@packages/core/src/screens/main/pos/cart/add-customer.tsx`:
- Line 93: Replace the current pattern that attempts to translate arbitrary
error text (the t('{message}', {message: error.message || 'Error' }) call in
add-customer.tsx) with a stable user-facing translation key (e.g.,
t('cart.addCustomerError') or similar) and display that to users; preserve the
original error for debugging by including error.message in the cartLogger.error
context or a separate log entry (i.e., call cartLogger.error('Add customer
failed', { error }) or add error to the metadata) so you don’t pass raw
technical text into the translator.
In `@packages/core/src/screens/main/settings/theme.tsx`:
- Around line 41-71: themeOptions is calling t(...) when setting descriptionKey
which pre-translates the text; change each descriptionKey to a raw translation
key string (e.g. "Follow your device settings", "Clean and bright", etc.)
instead of t('...') so it matches how labelKey is stored, and ensure the
component that renders themes uses the translator (the same place labelKey is
translated — see where labelKey is looked up around the render logic at
functions/components referencing themeOptions) to translate descriptionKey at
render time (like labelKey at lines referenced).
🧹 Nitpick comments (10)
packages/core/src/screens/main/page-upgrade.tsx (1)
29-33: Remove leftover empty objects from translation calls.The
descriptiontranslation calls have empty objects{}that appear to be leftovers from the_tagsremoval. These should be cleaned up for consistency with thetitlecalls.♻️ Proposed fix
case 'products': return { imageURL: 'https://wcpos.com/products-upgrade.png', title: t('Upgrade to Pro'), - description: t( - 'Adjust product prices and quantities by upgrading to WooCommerce POS Pro', - { - } - ), + description: t('Adjust product prices and quantities by upgrading to WooCommerce POS Pro'), demoURL: 'https://demo.wcpos.com/pos/products', }; case 'orders': return { imageURL: 'https://wcpos.com/orders-upgrade.png', title: t('Upgrade to Pro'), - description: t( - 'Re-open and print receipts for older orders by upgrading to WooCommerce POS Pro', - { - } - ), + description: t('Re-open and print receipts for older orders by upgrading to WooCommerce POS Pro'), demoURL: 'https://demo.wcpos.com/pos/orders', }; case 'customers': return { imageURL: 'https://wcpos.com/customers-upgrade.png', title: t('Upgrade to Pro'), - description: t( - 'Add new customers and edit existing customers by upgrading to WooCommerce POS Pro', - { - } - ), + description: t('Add new customers and edit existing customers by upgrading to WooCommerce POS Pro'), demoURL: 'https://demo.wcpos.com/pos/customers', }; case 'reports': return { imageURL: 'https://wcpos.com/reports-upgrade.png', title: t('Upgrade to Pro'), - description: t('Unlock End of Day Reports by upgrading to WooCommerce POS Pro', { - }), + description: t('Unlock End of Day Reports by upgrading to WooCommerce POS Pro'), demoURL: 'https://demo.wcpos.com/pos/reports', };Also applies to: 40-44, 51-55, 62-63
packages/core/src/screens/main/pos/cart/buttons/edit-order-meta/form.tsx (1)
89-96: Consider cleaning up or updating commented-out code.The commented-out code on lines 89 and 96 has been updated to remove
_tags, but if this code is intended to be re-enabled in the future, note that i18next uses double curly braces{{variable}}for interpolation by default, not single braces{variable}. The current syntaxt('Order #{number} saved', {number: ...})andt('{message}', {message: ...})would need adjustment.If this code is no longer needed, consider removing it entirely to reduce noise.
packages/core/src/screens/auth/hooks/use-url-discovery.ts (1)
129-141: Consider preserving the specific error message.The specific error message "Site does not seem to be a WordPress site" thrown at line 130 is replaced with a generic "Failed to discover WordPress API" in the catch block at line 138. This loses useful diagnostic information for the user.
♻️ Optional: Preserve specific error messages
if (!discoveredUrl) { - throw new Error(t('Site does not seem to be a WordPress site')); + const errorMessage = t('Site does not seem to be a WordPress site'); + setError(errorMessage); + setStatus('error'); + return null; } setWpApiUrl(discoveredUrl); setStatus('success'); discoveryLogger.debug(`WordPress API URL discovered: ${discoveredUrl}`); return discoveredUrl; - } catch { - const errorMessage = t('Failed to discover WordPress API'); + } catch (err) { + const errorMessage = err instanceof Error ? err.message : t('Failed to discover WordPress API'); setError(errorMessage); setStatus('error'); return null; }packages/core/src/screens/main/products/use-barcode.ts (1)
33-40: Consider using i18next's built-in pluralization.The current approach uses two separate translation keys for singular/plural cases. i18next supports automatic pluralization based on count, which handles complex plural rules for different languages.
♻️ Optional refactor using i18next pluralization
if (results.length === 1) { - text2 = t('1 product found locally'); - } - - if (results.length === 0 || results.length > 1) { - text2 = t('{count} products found locally', { count: results.length }); + text2 = t('product_found_locally', { count: 1 }); + } else { + text2 = t('product_found_locally', { count: results.length }); isError = true; }This requires adding plural keys to the translation files:
{ "product_found_locally_one": "{{count}} product found locally", "product_found_locally_other": "{{count}} products found locally" }packages/core/src/screens/main/pos/hooks/use-add-customer.ts (1)
68-68: Minor formatting inconsistency in object literal.The object
{customerName }has inconsistent spacing. Consider normalizing to{ customerName }for consistency with standard formatting conventions.✨ Suggested formatting fix
- orderLogger.success(t('Customer assigned: {customerName}', {customerName }), { + orderLogger.success(t('Customer assigned: {customerName}', { customerName }), {packages/core/src/screens/main/customers/cells/actions.tsx (1)
120-124: Remove empty options object from translation call.The empty
{}options object is unnecessary and appears to be a leftover from removing_tags: 'core'.🧹 Suggested cleanup
{t( 'Are you sure you want to delete this user? Deleting a user is permanent, there is no Trash for WordPress users.', - { - } )}packages/core/package.json (1)
42-43: Consider upgrading i18next and react-i18next to newer major versions.The specified versions (
i18next@^24.0.0andreact-i18next@^15.0.0) are valid and compatible with each other. However, both packages have newer major releases available: i18next 25.8.0 and react-i18next 16.5.4. If there are no constraints preventing the upgrade, consider using the latest versions for better bug fixes, performance improvements, and security updates.packages/core/src/screens/main/logs/filter-bar.tsx (1)
19-25: Consider translating the log level labels.The
itemsarray contains hardcoded English labels ('Error','Warning','Info','Success','Audit') that are not wrapped int()calls, unlike most other UI strings in this PR. If these should be localized, they would need translation wrappers.Example if translation is desired
+const useItems = () => { + const t = useT(); + return React.useMemo(() => [ + { value: 'error', label: t('Error') }, + { value: 'warn', label: t('Warning') }, + { value: 'info', label: t('Info') }, + { value: 'success', label: t('Success') }, + { value: 'audit', label: t('Audit') }, + ], [t]); +};packages/core/src/screens/main/products/edit/variation/form.tsx (2)
114-114: Verify the translation pattern for error messages.The translation key
'{message}'with interpolation{message: error.message || 'Error'}will just pass through the raw error message without actual translation. Iferror.messagecontains technical or English-only text, it won't be localized.If the intent is to display the raw error message, consider removing the
t()wrapper:-mutationLogger.error(t('{message}', {message: error.message || 'Error' }), { +mutationLogger.error(error.message || t('Error'), {Alternatively, if error messages should be translated, use a more specific translation key with the error as context.
63-63: Consider avoiding translated strings in thrown errors.Throwing an error with a translated message (
throw new Error(t('Variation not found'))) may cause issues with error tracking systems that group errors by message. Additionally, developers debugging in different locales may see localized error messages.Consider keeping error messages in English and translating only at the display layer, or using error codes:
-throw new Error(t('Variation not found')); +throw new Error('Variation not found');
- Convert 'product found locally' strings to use i18next plural keys (product_found_locally_one/other) instead of conditional singular/plural - Convert 'variation found for term' strings similarly - Translation files should provide _one and _other suffixed keys for proper support of languages with multiple plural forms (Russian, Arabic, etc.) - Fix prettier formatting issues caught by linter - Update pnpm-lock.yaml with i18next dependencies
📊 Test Coverage Report
Coverage Legend
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/screens/auth/hooks/use-api-discovery.ts (1)
116-123:⚠️ Potential issue | 🟠 MajorError re-throw logic will break for non-English locales.
The checks on lines 119-120 compare
error.messageagainst the English source strings ('Bad API response','WordPress API not found'). After i18next translates these, the comparison will fail for any non-English locale, causing the catch block to re-log and re-throw already-handled errors.Consider using a custom error class or error code property instead of string matching:
🔧 Proposed fix using custom error class
+class ApiDiscoveryError extends Error { + constructor(message: string) { + super(message); + this.name = 'ApiDiscoveryError'; + } +} + const fetchApiIndex = React.useCallback( async (wpApiUrl: string): Promise<WpJsonResponse> => { try { // ... if (!data || typeof data !== 'object') { // ... - throw new Error(t('Bad API response')); + throw new ApiDiscoveryError(t('Bad API response')); } // ... if (!namespaces || !Array.isArray(namespaces)) { // ... - throw new Error(t('WordPress API not found')); + throw new ApiDiscoveryError(t('WordPress API not found')); } // ... } catch (error) { - if ( - error.message.includes('Bad API response') || - error.message.includes('WordPress API not found') - ) { + if (error instanceof ApiDiscoveryError) { throw error; } // Handle network/connection errors // ... } }, [http, t] );
🧹 Nitpick comments (2)
packages/core/src/screens/auth/hooks/use-api-discovery.ts (1)
212-214: Remove unnecessary empty object argument.The empty object
{}passed tot()serves no purpose and is inconsistent with all othert()calls in this file. This appears to be a remnant from the migration.🧹 Proposed fix
throw new Error( - t('WCPOS login URL is invalid. Please ensure WCPOS plugin is properly configured', {}) + t('WCPOS login URL is invalid. Please ensure WCPOS plugin is properly configured') );packages/core/src/screens/main/products/edit/variation/form.tsx (1)
114-122: Unnecessary translation wrapper around dynamic error message.Using
t('{message}', { message: error.message || 'Error' })doesn't translate anything meaningful—it just passes through the interpolated value. Theerror.messagecontent won't be translated since it's a dynamic runtime value.Consider using the error message directly or providing a static translated fallback:
Suggested fix
- mutationLogger.error(t('{message}', { message: error.message || 'Error' }), { + mutationLogger.error(error.message || t('Failed to save variation'), {
- Add wcposVersion to useMemo deps in TranslationProvider so the backend
fetches from the correct CDN URL when the version changes
- Use ApiDiscoveryError class instead of string comparison for error
re-throw logic (breaks in non-English locales)
- Store theme descriptionKey as raw strings, translate at render time
(consistent with labelKey pattern)
- Fix "effect" → "affect" typo in user-facing copy
- Replace t('{message}', { message: error.message }) pattern with static
translated messages (e.g. t('Failed to save product')) across 10 files
- Translate log level labels in filter-bar
- Preserve specific error messages in use-url-discovery catch block
- Clean up leftover empty {} from t() calls
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/screens/main/orders/edit/form.tsx (1)
205-213:⚠️ Potential issue | 🔴 Critical
errorMessageis not defined in this scope — will cause ReferenceError.The catch block at line 205 uses
errorMessage(line 210) but never defines it. This will throw aReferenceErrorat runtime when a customer fetch fails.🐛 Proposed fix
} catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); mutationLogger.error('Error fetching customer', { context: { errorCode: ERROR_CODES.RECORD_NOT_FOUND, customerId, error: errorMessage, }, }); }
🤖 Fix all issues with AI agents
In `@packages/core/src/contexts/translations/index.tsx`:
- Around line 12-50: The RxDBBackend currently calls the global i18next
singleton in read(), so background updates don't reach the instance created by
TranslationProvider; modify init() to save the passed services object (e.g.,
this.services = _services) and in read() replace i18next.addResourceBundle(...)
with the instance-scoped call
this.services.resourceStore.addResourceBundle(language, namespace, data, true,
true) (or the appropriate addResourceBundle on the saved services), ensuring you
still check and update this.translationsState via
this.translationsState.set(language, ...) as before.
- Use instance resourceStore instead of global i18next singleton in both RxDBBackend and ElectronStoreBackend, so background CDN fetches update the correct i18n store - Remove dead commented-out code and orphaned imports in edit-order-meta form
- Use named import { createInstance } instead of default i18next import
- Add locale to useMemo and useEffect dependency arrays
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/core/src/contexts/translations/index.tsx`:
- Around line 69-71: The i18next init options currently set fallbackLng: false
which disables any fallback language; change the init options object (the one
containing lng: locale, fallbackLng, ns: ['core']) to use fallbackLng: 'en' so
missing translations fall back to English, and keep the other keys (lng and ns)
unchanged; ensure this replacement is applied where the i18n initialization
occurs (the object with keys lng, fallbackLng, ns).
🧹 Nitpick comments (1)
packages/core/src/contexts/translations/index.tsx (1)
63-66: Avoid re-creating the i18n instance on every locale change.
changeLanguagealready handles locale changes; rebuilding the instance can reset resource store and event bindings. Consider keeping the instance stable by removinglocalefrom the memo deps.♻️ Suggested refactor
- }, [locale, translationsState, wcposVersion]); + }, [translationsState, wcposVersion]);Also applies to: 86-86
| lng: locale, | ||
| fallbackLng: false, | ||
| ns: ['core'], |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
i18next fallbackLng false documentation behavior English fallback
💡 Result:
- In i18next,
fallbackLngdefaults to"dev"(developer language). So if you don’t setfallbackLng, you may see fallback behavior that is not English. [1][2][3] - Setting
fallbackLng: "en"(or an ordered list like["fr","en"]) enables fallback to English (or your specified chain) when a key is missing in the active language. [1][3] - Setting
fallbackLng: falseturns the fallback language feature off: i18next will not load any fallback language resource files and will not look up missing keys in a fallback language. [1][2] - Note: even with
fallbackLng: false, i18next can still do variant resolution (e.g.,en-GB→en) unless you also constrain loading/behavior via options likeload. [3]
Sources: i18next init docs and configuration options, plus i18next fallback principles docs. [1][2][3]
🏁 Script executed:
cat -n packages/core/src/contexts/translations/index.tsx | head -80 | tail -30Repository: wcpos/monorepo
Length of output: 1033
fallbackLng: false disables fallback entirely, preventing English fallback from working.
Setting fallbackLng: false turns off the fallback language feature in i18next — it will not load fallback language resources or look up missing keys in a fallback language. This conflicts with the PR objective of confirming English fallback works when translations are unavailable.
Configure it as fallbackLng: 'en' to enable proper English fallback:
🛠️ Suggested fix
- fallbackLng: false,
+ fallbackLng: 'en',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| lng: locale, | |
| fallbackLng: false, | |
| ns: ['core'], | |
| lng: locale, | |
| fallbackLng: 'en', | |
| ns: ['core'], |
🤖 Prompt for AI Agents
In `@packages/core/src/contexts/translations/index.tsx` around lines 69 - 71, The
i18next init options currently set fallbackLng: false which disables any
fallback language; change the init options object (the one containing lng:
locale, fallbackLng, ns: ['core']) to use fallbackLng: 'en' so missing
translations fall back to English, and keep the other keys (lng and ns)
unchanged; ensure this replacement is applied where the i18n initialization
occurs (the object with keys lng, fallbackLng, ns).
- Add missing errorMessage declaration in handleCustomerChange catch block (was referencing variable from a different function scope) - Fix orders E2E test: use rowgroup selector instead of filtering by status text that was never present in the row DOM (tooltip portals to body) - Add comment clarifying why fallbackLng is false
Summary
@transifex/nativeand@transifex/reactwithi18nextandreact-i18nextRxDBBackendi18next plugin fetches translations from jsDelivr CDN and caches in RxDBsite.wcpos_versionso CDN URLs match the installed plugin releaseElectronStoreBackendfor the electron app with electron-store caching_tagsfrom allt()calls across 166+ files (namespace set viadefaultNSconfig)cache.tsfiles superseded by backend pluginsTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
User-facing
Bug Fixes