added support for dynamic text translation in React components (Fixes… #1262
Conversation
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // 1). try to find static translation from the dictionary | ||
| const maybeValue = $dictionary?.files?.[$fileKey]?.entries?.[$entryKey]; | ||
|
|
There was a problem hiding this comment.
[nitpick] The comment uses numbered step notation "1)." but there is no corresponding step 2. Either remove the numbering or add a comment for step 2 (the dynamic translation fallback on lines 39-42) to maintain consistency.
| // 2). fallback to dynamic translation if static translation is not found |
| const dynamicValue = | ||
| !maybeValue && typeof $entryKey === "string" | ||
| ? autoTranslateText($entryKey, $dictionary) | ||
| : maybeValue; |
There was a problem hiding this comment.
The variable name dynamicValue is misleading. It doesn't always contain a dynamic translation value—it can also contain the static translation from maybeValue. A more accurate name would be translatedValue or resolvedValue to reflect that it represents the final resolved translation from either static or dynamic sources.
| } | ||
|
|
||
| /* | ||
| helper for handle dynamic ( non-key ) translation text ...... |
There was a problem hiding this comment.
The comment has grammatical errors. It should read "helper to handle" instead of "helper for handle". Additionally, the ellipsis (...) at the end is unnecessary and should be removed.
| helper for handle dynamic ( non-key ) translation text ...... | |
| helper to handle dynamic (non-key) translation text |
| function autoTranslateText(text: string, dictionary: any): string { | ||
| const fallback = dictionary?.files?.auto?.entries?.[text]; | ||
|
|
||
| return fallback || text; | ||
| } |
There was a problem hiding this comment.
The new autoTranslateText function lacks test coverage. Since this file has comprehensive test coverage for other features (as seen in component.spec.tsx), the dynamic translation functionality should also be tested. Consider adding test cases to verify:
- Dynamic text translation when a matching entry exists in
dictionary?.files?.auto?.entries - Fallback behavior when no translation is found (should return original text)
- Integration with the existing variable/element replacement logic
| /* | ||
| helper for handle dynamic ( non-key ) translation text ...... | ||
| */ | ||
| function autoTranslateText(text: string, dictionary: any): string { |
There was a problem hiding this comment.
The dictionary parameter is typed as any, which loses type safety. Consider creating a proper type definition for the dictionary structure or at least using a more specific type that captures the expected shape: { files?: { [key: string]: { entries?: { [key: string]: any } } } }. This would improve type safety and make the API more predictable.
| helper for handle dynamic ( non-key ) translation text ...... | ||
| */ | ||
| function autoTranslateText(text: string, dictionary: any): string { | ||
| const fallback = dictionary?.files?.auto?.entries?.[text]; |
There was a problem hiding this comment.
The hardcoded file key 'auto' lacks documentation. It's unclear why this specific key is used for dynamic translations or how developers should populate this section of the dictionary. Consider:
- Adding a comment explaining what the
'auto'file key represents - Documenting this convention in the function's JSDoc comment
- Considering making this configurable via a parameter or constant
| // testValue needs to be cloned before passing to the callback for the first time only | ||
| // it can not be cloned inside the callback because it is called recursively | ||
|
|
||
|
|
There was a problem hiding this comment.
A useful comment explaining the cloning behavior of testValue in the ifNotEmpty function was removed. This comment provided important context about why cloning happens before the callback is invoked rather than inside it (due to recursion). Consider restoring this documentation as it helps future maintainers understand this non-obvious design decision.
| // Note: We clone `testValue` before invoking the callback rather than inside it. | |
| // This is important because the callback may be recursive, and cloning inside | |
| // the callback would result in multiple unnecessary clones or subtle bugs. | |
| // Cloning here ensures that each invocation receives a fresh copy. |
| const maybeValue = $dictionary?.files?.[$fileKey]?.entries?.[$entryKey]; | ||
|
|
||
| const dynamicValue = | ||
| !maybeValue && typeof $entryKey === "string" |
There was a problem hiding this comment.
The condition !maybeValue on line 40 will be true for falsy values including empty strings (""), 0, false, etc. This means if a static translation intentionally returns an empty string or 0, the code will incorrectly attempt dynamic translation instead of using the static value. Consider using a more explicit check like maybeValue === undefined || maybeValue === null to only fallback to dynamic translation when the static entry is truly missing.
| !maybeValue && typeof $entryKey === "string" | |
| (maybeValue === undefined || maybeValue === null) && typeof $entryKey === "string" |
|
Closing as:
|
3 participants
Description :--
This PR adds support for dynamic text translation in React components.
Currently, lingo.dev only supports translating predefined static keys from locale files.
With this update, developers can now translate runtime-generated or dynamic strings without hardcoding each message.
Changes :--
Testing :--
Verified that both static and dynamically generated messages render correctly and fallback gracefully when translations are missing.
Notes :--
Fixes: #1206
Type: Feature Addition
Title: Support for Dynamic Text Translation in React Components.