-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Localization: Adds termOrDefault() method to accept a fallback value
#20947
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
base: main
Are you sure you want to change the base?
Conversation
…if the translation does not exist
…te' to contain number of re-renders
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.
Pull request overview
This PR introduces a new termOrDefault() method to the UmbLocalizationController that provides a cleaner way to detect missing translations by accepting a fallback value, mirroring Umbraco's C# pattern. The implementation refactors the existing term() method to extract common logic into two private helper methods (#lookupTerm() and #processTerm()), improving code reusability. Additionally, the localization elements (umb-localize, umb-localize-number, umb-localize-relative-time) were updated to use the new method and the when() directive for more efficient conditional rendering.
Key Changes:
- Added
termOrDefault()method that returns a specified default value instead of the key when translation is not found - Refactored localization lookup and processing into reusable private helper methods
- Updated localization elements to use
when()directive to prevent double getter evaluation
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
localization.controller.ts |
Added termOrDefault() method and refactored term() and string() methods to use extracted #lookupTerm() and #processTerm() helpers |
localization.controller.test.ts |
Added 8 comprehensive tests covering termOrDefault() scenarios including missing keys, arguments, function-based translations, and fallback resolution |
localize.element.ts |
Updated to use termOrDefault(key, null) for cleaner null checking and when() directive for rendering |
localize-number.element.ts |
Changed to use when() directive and added explicit type: Object for options property |
localize-relative-time.element.ts |
Changed to use when() directive for consistent rendering pattern |
src/Umbraco.Web.UI.Client/src/packages/core/localization/localize-number.element.ts
Outdated
Show resolved
Hide resolved
src/Umbraco.Web.UI.Client/src/packages/core/localization/localize-relative-time.element.ts
Outdated
Show resolved
Hide resolved
src/Umbraco.Web.UI.Client/src/libs/localization-api/localization.controller.ts
Show resolved
Hide resolved
src/Umbraco.Web.UI.Client/src/libs/localization-api/localization.controller.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Add
termOrDefault()Method and Optimize Localization ElementsSummary
This PR adds a new
termOrDefault()method toUmbLocalizationControllerthat provides a cleaner way to detect missing translations, matching Umbraco's C# pattern. Additionally, we explored several performance optimization approaches for the localization elements and ultimately reverted to the original getter pattern, which proved to be the most performant solution.What We Changed
1. Added
termOrDefault()Method toUmbLocalizationControllerMotivation: The previous pattern for detecting missing translations required comparing the result to the key itself:
This mirrors Umbraco's C# pattern where
dictionaryValuereturns the key if missing, anddictionaryValueOrDefaultreturns a specified default value.New Pattern:
Implementation: Added helper methods to avoid code duplication:
Generic Type Constraint: The method signature uses
D extends string | nullto allow both string and null default values while maintaining type safety.2. Refactored Localization Elements to Use Getter Pattern
All three localization elements now use a consistent, simple getter pattern:
Changed Files:
localize.element.ts- Now usestermOrDefault(key, null)for cleaner null checkinglocalize-number.element.ts- Reverted fromwillUpdate()pattern to getterlocalize-relative-time.element.ts- Reverted fromwillUpdate()pattern to getter3. Added Comprehensive Tests
Added 8 new tests for
termOrDefault()method covering various scenarios:Performance Optimization Attempts
We explored several approaches to optimize the localization elements, aiming to reduce computational overhead during rendering.
Approach 1:
willUpdate()Lifecycle Hook with CachingTheory: Cache the computed localized value in a state variable using the
willUpdate()lifecycle hook, avoiding recomputation on every render.Implementation:
Results:
Why It Failed: The lifecycle overhead (
willUpdate()→ state update →updated()→ render) outweighed any benefit from caching. For simple computations like number formatting, the Lit lifecycle machinery adds more cost than the computation itself.Approach 2: Inline Ternary with Getter
Issue Discovered: When using inline ternary, the getter was accessed twice:
Solution: Use Lit's
when()directive, which evaluates the condition once and caches the result:Results: Reduced getter calls from 6 to 3 for 3 property changes (50% reduction).
Approach 3: Side Effects in
willUpdate()Concern: The getter pattern has side effects (DOM attribute manipulation), which is technically an anti-pattern.
Attempted Fix: Move side effects to
willUpdate():Results: This caused the getter to be called twice (once by
willUpdate(), once byrender()), doubling the computational cost.Conclusion: Side effects in the getter are acceptable for this use case since:
@state(), signaling reactive behaviorFinal Performance Comparison
After testing all approaches, the original getter pattern performed best:
The final implementation matches main branch performance while providing cleaner code through
termOrDefault().Key Takeaways
when()directive: Prevents double getter access in conditional rendering.termOrDefault()instead of string comparison makes the code more readable without any performance penalty.Files Changed
src/libs/localization-api/localization.controller.ts- AddedtermOrDefault(),#lookupTerm(),#processTerm()src/libs/localization-api/localization.controller.test.ts- Added 8 comprehensive testssrc/packages/core/localization/localize.element.ts- Updated to usetermOrDefault()src/packages/core/localization/localize-number.element.ts- Reverted to getter patternsrc/packages/core/localization/localize-relative-time.element.ts- Reverted to getter pattern