Skip to content

added support for dynamic text translation in React components (Fixes… #1262

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.

Sign up for GitHub

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

Closed
SaketTiwari2008 wants to merge 3 commits into from
Closed

added support for dynamic text translation in React components (Fixes… #1262

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d4ad686
added support for dynamic text translation in React components (Fixes…
SaketTiwari2008 Oct 29, 2025
b20cf51
Merge branch 'main' into feature/dynamic-text-translation
SaketTiwari2008 Oct 30, 2025
8d2819d
Merge branch 'main' into feature/dynamic-text-translation
sumitsaurabh927 Nov 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions packages/react/src/core/component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,24 @@ export const LingoComponent = React.forwardRef(
$expressions,
...rest
} = props;
const maybeValue = $dictionary?.files?.[$fileKey]?.entries?.[$entryKey];

// 1). try to find static translation from the dictionary
const maybeValue = $dictionary?.files?.[$fileKey]?.entries?.[$entryKey];

Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

[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.

Suggested change
// 2). fallback to dynamic translation if static translation is not found

Copilot uses AI. Check for mistakes.
const dynamicValue =
!maybeValue && typeof $entryKey === "string"
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
!maybeValue && typeof $entryKey === "string"
(maybeValue === undefined || maybeValue === null) && typeof $entryKey === "string"

Copilot uses AI. Check for mistakes.
? autoTranslateText($entryKey, $dictionary)
: maybeValue;
Comment on lines +39 to +42
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.

const children = useMemo(() => {
return _.flow([
(nodes) => ifNotEmpty(replaceElements, $elements, nodes),
(nodes) => ifNotEmpty(replaceVariables, $variables, nodes),
(nodes) => ifNotEmpty(replaceFunctions, $functions, nodes),
(nodes) => ifNotEmpty(replaceExpressions, $expressions, nodes),
])([maybeValue ?? $entryKey]);
])([dynamicValue ?? $entryKey]);
}, [
maybeValue,
dynamicValue,
$entryKey,
$elements,
$variables,
Expand All @@ -65,8 +72,8 @@ export const LingoComponent = React.forwardRef(
},
);

// 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


Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
// 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.

Copilot uses AI. Check for mistakes.
function ifNotEmpty<T>(
callback: (nodes: ReactNode[], value: T) => ReactNode[],
testValue: T,
Expand Down Expand Up @@ -284,3 +291,12 @@ function replaceExpressions(

return processWithIndex(nodes);
}

/*
helper for handle dynamic ( non-key ) translation text ......
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
helper for handle dynamic ( non-key ) translation text ......
helper to handle dynamic (non-key) translation text

Copilot uses AI. Check for mistakes.
*/
function autoTranslateText(text: string, dictionary: any): string {
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
const fallback = dictionary?.files?.auto?.entries?.[text];
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

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:

  1. Adding a comment explaining what the 'auto' file key represents
  2. Documenting this convention in the function's JSDoc comment
  3. Considering making this configurable via a parameter or constant

Copilot uses AI. Check for mistakes.

return fallback || text;
}
Comment on lines +298 to +302
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

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:

  1. Dynamic text translation when a matching entry exists in dictionary?.files?.auto?.entries
  2. Fallback behavior when no translation is found (should return original text)
  3. Integration with the existing variable/element replacement logic

Copilot uses AI. Check for mistakes.
Loading