Skip to content

V16: Localization extensions load unordered#19474

Merged
leekelleher merged 12 commits intorelease/16.0from
v16/bugfix/localization-extensions-out-of-order
Jun 3, 2025
Merged

V16: Localization extensions load unordered#19474
leekelleher merged 12 commits intorelease/16.0from
v16/bugfix/localization-extensions-out-of-order

Conversation

@iOvergaard
Copy link
Contributor

@iOvergaard iOvergaard commented Jun 3, 2025

Description

This pull request is difficult to explain, but in summary it fixes #19464.

It makes two changes to the UmbLocalizationRegistry:

First change: Do not check if localizations are already loaded; load them always when something changes

Old Logic (with alias/diff check): Risk of Stale/Un-overridden Translations

flowchart TD
    A[Language Change or Extension Registered] --> B[Check extension alias against loaded list]
    B -- Alias already loaded --> C[Skip registration]
    B -- Alias not loaded --> D[Register translations]
    D --> E[Translations in localization manager]
    C --> E
    E --> F[Result: Some keys may NOT be overridden if new extension has lower weight]
Loading

New Logic (Always Register All, Sorted by Weight): Reliable Overrides

flowchart TD
    A[Language Change or Extension Registered] --> B[Collect all matching extensions]
    B --> C[Sort all by weight, lowest weight = highest priority]
    C --> D[Register ALL translations in order]
    D --> E[Translations in localization manager]
    E --> F[Result: Lower weight always overrides higher, keys are reliably overridden]
Loading

Second change: Use the power of rxjs with switchMap to ensure that localizations are loaded in the order they are received

For example, if we imagined a user would register the extensions like en -> en-us -> en-us -> en-us, we would want all of them to finish, e.g. they could register their own keys each and override previous keys as well:

Old Logic (with combineLatest)

sequenceDiagram
    participant User
    participant Registry
    participant Loader

    User->>Registry: Registers "en"
    Registry->>Loader: Start loading "en"
    User->>Registry: Quickly registers "en-us"
    Registry->>Loader: Start loading "en, en-us" (queued)
    User->>Registry: Quickly registers "en-us" again
    Registry->>Loader: Start loading "en, en-us, en-us" (queued)
    User->>Registry: Quickly registers "en-us" a third time
    Registry->>Loader: Start loading "en, en-us, en-us, en-us" (queued)
    Loader-->>Registry: "en" finishes (may override)
    Loader-->>Registry: "en, en-us" finishes (may override)
    Loader-->>Registry: "en, en-us, en-us" finishes (may override)
    Loader-->>Registry: "en, en-us, en-us, en-us" finishes (final state)
    Note right of Registry: All loads run in order, but slow loads may override newer ones if not careful, so that the "en, en-us" could be final state.
Loading

New Logic (with switchMap, cancellation of previous loads)

sequenceDiagram
    participant User
    participant Registry
    participant Loader

    User->>Registry: Registers "en"
    Registry->>Loader: Start loading "en"
    User->>Registry: Quickly registers "en-us"
    Registry--xLoader: Cancel loading "en"
    Registry->>Loader: Start loading "en, en-us"
    User->>Registry: Quickly registers "en-us" again
    Registry--xLoader: Cancel loading "en, en-us"
    Registry->>Loader: Start loading "en, en-us, en-us"
    User->>Registry: Quickly registers "en-us" a third time
    Registry--xLoader: Cancel loading "en, en-us, en-us"
    Registry->>Loader: Start loading "en, en-us, en-us, en-us"
    Loader-->>Registry: "en, en-us, en-us, en-us" is allowed to finish (final state)
    Note right of Registry: Only the latest language set ("en, en-us, en-us, en-us") is loaded and applied and in that weighted order.
Loading

Additionally, Copilot pointed out that if the user switches really fast between languages (for example en -> da -> de), and one of them loads through an async file or otherwise is delayed, it could actually mean that even though it would end up on "de", it would possibly apply the "da" translations. Now, they would never be shown to the user (wrong iso code), but it would possibly not have loaded in the custom translations from "de" in that case.

How to test

  1. Register a bunch of localization extensions with the same culture code, e.g. "en-us"
  2. Make sure you have the same key in most or all of them with different values
  3. Apply a "weight" to them, and test through <umb-localize key="your_key"></umb-localize> that the lowest weight wins
  4. A good case to test is localization overrides on the login screen

This comment was marked as outdated.

@iOvergaard iOvergaard requested a review from Copilot June 3, 2025 14:58

This comment was marked as outdated.

@iOvergaard iOvergaard requested a review from Copilot June 3, 2025 15:12
Copy link
Contributor

Copilot AI left a 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 improves the loading of localization extensions by always reloading them and ensuring that translations are applied in the correct priority order using rxjs’s switchMap for cancellation of outdated loads. Key changes include:

  • Refactoring the localization registry to remove previously loaded alias checks and always reload extensions.
  • Introducing a new observable pipeline with switchMap, distinctUntilChanged, and filter to cancel stale translation loads.
  • Enhancing tests to validate asynchronous overrides and correct language/direction setting.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/packages/core/localization/registry/localization.registry.ts Refactored to use a new rxjs pipeline ensuring correct ordering & cancellation and removing loaded alias checks; added subscription handling and array comparison helper.
src/packages/core/localization/registry/localization.registry.test.ts Updated tests to register multiple extensions at once and test weight-based translation overrides.
src/packages/core/localization/localize.element.test.ts Added tests for async localization overrides and fallback language scenarios; updated test data structures.
src/external/rxjs/index.ts Added re-exports for additional rxjs operators to support new registry logic.

Copy link
Member

@leekelleher leekelleher left a comment

Choose a reason for hiding this comment

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

Amazing description and flow charts! 🤩

Tested out and it works as expected! 🚀

@leekelleher leekelleher enabled auto-merge (squash) June 3, 2025 16:09
@leekelleher leekelleher merged commit 8a22f24 into release/16.0 Jun 3, 2025
23 checks passed
@leekelleher leekelleher deleted the v16/bugfix/localization-extensions-out-of-order branch June 3, 2025 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants