Skip to content

Commit 441672b

Browse files
nnaydenownnaydenow
authored
fix(framework): prevent language-aware components from rendering before CLDR data loads (#13602)
When `setLanguage()` is called without awaiting, `languageChangePending` is set to true while CLDR/i18n data is still being fetched. Previously, a language-aware component mounted during this window would call `renderImmediately` with incomplete locale data, causing a crash. The fix registers the element in `connectedCallback` before any async work, then bails out of rendering if a language change is pending. Once CLDR resolves, `reRenderAllUI5Elements({ languageAware: true })` picks up the registered element and renders it correctly. `registerElement`/`unregisterElement` are extracted from `renderImmediately` and `cancelRender` and exported so `UI5Element` can call them independently of the render cycle.
1 parent b9c8607 commit 441672b

4 files changed

Lines changed: 230 additions & 2 deletions

File tree

‎packages/base/src/Render.ts‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,28 @@ const renderDeferred = async (webComponent: UI5Element) => {
3333
await scheduleRenderTask();
3434
};
3535

36+
/**
37+
* Register all web components attached to the DOM
38+
*/
39+
const registerElement = (webComponent: UI5Element) => {
40+
registeredElements.add(webComponent);
41+
};
42+
43+
/**
44+
* Unregister all web components detached from the DOM
45+
*/
46+
const unregisterElement = (webComponent: UI5Element) => {
47+
registeredElements.delete(webComponent);
48+
};
49+
3650
/**
3751
* Renders a component synchronously and adds it to the registry of rendered components
3852
*
3953
* @param webComponent
4054
*/
4155
const renderImmediately = (webComponent: UI5Element) => {
4256
eventProvider.fireEvent("beforeComponentRender", webComponent);
43-
registeredElements.add(webComponent);
57+
registerElement(webComponent);
4458
webComponent._render();
4559
};
4660

@@ -51,7 +65,7 @@ const renderImmediately = (webComponent: UI5Element) => {
5165
*/
5266
const cancelRender = (webComponent: UI5Element) => {
5367
invalidatedWebComponents.remove(webComponent);
54-
registeredElements.delete(webComponent);
68+
unregisterElement(webComponent);
5569
};
5670

5771
/**
@@ -173,6 +187,8 @@ export {
173187
renderDeferred,
174188
renderImmediately,
175189
cancelRender,
190+
registerElement,
191+
unregisterElement,
176192
renderFinished,
177193
reRenderAllUI5Elements,
178194
attachBeforeComponentRender,

‎packages/base/src/UI5Element.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import {
1818
renderDeferred,
1919
renderImmediately,
2020
cancelRender,
21+
unregisterElement,
22+
registerElement,
2123
} from "./Render.js";
2224
import { registerTag, isTagRegistered, recordTagRegistrationFailure } from "./CustomElementsRegistry.js";
2325
import { observeDOMNode, unobserveDOMNode } from "./DOMObserver.js";
@@ -332,6 +334,8 @@ abstract class UI5Element extends HTMLElement {
332334

333335
const ctor = this.constructor as typeof UI5Element;
334336

337+
registerElement(this);
338+
335339
this.setAttribute(ctor.getMetadata().getPureTag(), "");
336340
if (ctor.getMetadata().supportsF6FastNavigation() && !this.hasAttribute("data-sap-ui-fastnavgroup")) {
337341
this.setAttribute("data-sap-ui-fastnavgroup", "true");
@@ -351,6 +355,12 @@ abstract class UI5Element extends HTMLElement {
351355
await ctor._definePromise;
352356
}
353357

358+
// Skip rendering while a language change is in progress to avoid rendering with not fully loaded locale data.
359+
// Once the locale data is loaded, the language-aware component will be re-rendered.
360+
if (ctor.getMetadata().isLanguageAware() && getLanguageChangePending()) {
361+
return;
362+
}
363+
354364
if (!this._inDOM) { // Component removed from DOM while _processChildren was running
355365
return;
356366
}
@@ -401,6 +411,7 @@ abstract class UI5Element extends HTMLElement {
401411
this._domRefReadyPromise._deferredResolve!();
402412

403413
cancelRender(this);
414+
unregisterElement(this);
404415
}
405416

406417
/**

‎packages/main/cypress/specs/DatePicker_cldr_race.cy.tsx‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import "../../src/Assets.js";
2+
import { setLanguage } from "@ui5/webcomponents-base/dist/config/Language.js";
3+
import DatePicker from "../../src/DatePicker.js";
4+
5+
// Timestamps for Feb 2019 calendar checks
6+
const timestamp_3_Feb_2019 = 1549152000;
7+
const timestamp_28_Jan_2019 = 1548633600;
8+
9+
describe("DatePicker CLDR race condition", () => {
10+
it("mounts correctly when setLanguage is called without await before mount", () => {
11+
// Capture the promise but do NOT await it — this is the race condition.
12+
// Before the fix, the component rendered before CLDR data loaded → crash.
13+
// After the fix, connectedCallback bails early and reRenderAllUI5Elements
14+
// picks up the registered element once CLDR resolves.
15+
let languageReady: Promise<void>;
16+
cy.then(() => {
17+
languageReady = setLanguage("bg");
18+
});
19+
20+
cy.mount(<DatePicker value="фев 6, 2019" formatPattern="MMM d, y"></DatePicker>);
21+
22+
// Now wait for the language change to settle so locale data is applied
23+
cy.then(() => languageReady);
24+
25+
cy.get("[ui5-date-picker]")
26+
.as("datePicker")
27+
.ui5DatePickerValueHelpIconPress();
28+
29+
// Monday (bg locale) should be the first displayed date — week starts Jan 28
30+
cy.get<DatePicker>("@datePicker")
31+
.ui5DatePickerGetFirstDisplayedDate()
32+
.should("have.attr", "data-sap-timestamp", timestamp_28_Jan_2019.toString());
33+
34+
// Feb 3 should fall on Sunday (last day in Mon-first week = wday6)
35+
cy.get<DatePicker>("@datePicker")
36+
.ui5DatePickerGetPopoverDate(timestamp_3_Feb_2019)
37+
.should("have.class", "ui5-dp-wday6");
38+
});
39+
});

‎packages/main/test/pages/DatePicker_cldr_race.html‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8">
5+
<meta name="viewport" content="width=device-width, initial-scale=1.0">
6+
<title>DatePicker — CLDR race condition reproduction</title>
7+
8+
<script src="%VITE_BUNDLE_PATH%" type="module"></script>
9+
10+
<style>
11+
body { font-family: sans-serif; padding: 1rem; }
12+
section { margin-bottom: 2rem; border: 1px solid #ccc; padding: 1rem; border-radius: 4px; }
13+
h2 { margin-top: 0; }
14+
button { margin: 0.25rem; padding: 0.4rem 0.8rem; cursor: pointer; }
15+
pre { background: #f4f4f4; padding: 0.5rem; border-radius: 3px; white-space: pre-wrap; font-size: 0.85rem; }
16+
.ok { color: green; font-weight: bold; }
17+
.fail { color: red; font-weight: bold; }
18+
</style>
19+
</head>
20+
<body>
21+
<h1>DatePicker — CLDR race condition</h1>
22+
<p>
23+
Reproduces the bug where <code>getCachedLocaleDataInstance</code> creates a <code>LocaleData</code>
24+
instance before CLDR data has finished loading, leaving <code>mData</code> undefined and
25+
crashing on first render.
26+
</p>
27+
28+
<section>
29+
<h2>Scenario 1 — setLanguage("bg") then immediately mount</h2>
30+
<p>
31+
Calls <code>setLanguage("bg")</code> <strong>without awaiting</strong> it, then immediately
32+
adds a <code>&lt;ui5-date-picker&gt;</code>. The race: the component renders before
33+
<code>fetchCldr("bg")</code> resolves, so <code>mData</code> is undefined → crash.<br>
34+
<em>Click the button to trigger the race.</em>
35+
</p>
36+
<button id="btn-race">Trigger race (no await)</button>
37+
<button id="btn-safe">Trigger safe (await setLanguage)</button>
38+
<button id="btn-reset">Reset (remove picker, set EN)</button>
39+
<div id="container1" style="margin-top:1rem"></div>
40+
<pre id="log1">— click a button —</pre>
41+
</section>
42+
43+
<section>
44+
<h2>Scenario 2 — language already set, normal mount</h2>
45+
<p>
46+
After <code>setLanguage("bg")</code> has fully resolved (awaited), mounting a new picker
47+
should work correctly and show Monday as first day of week.
48+
</p>
49+
<button id="btn-scenario2">Mount after await</button>
50+
<button id="btn-open2">Open picker</button>
51+
<div id="container2" style="margin-top:1rem"></div>
52+
<pre id="log2">— click "Mount after await" —</pre>
53+
</section>
54+
55+
<section>
56+
<h2>Scenario 3 — rapid language switches</h2>
57+
<p>
58+
Switches language multiple times in quick succession without awaiting each call,
59+
then mounts. Stresses the cache-poisoning path.
60+
</p>
61+
<button id="btn-rapid">Rapid switch + mount</button>
62+
<div id="container3" style="margin-top:1rem"></div>
63+
<pre id="log3">— click "Rapid switch + mount" —</pre>
64+
</section>
65+
66+
<script type="module">
67+
import { setLanguage } from "../../../base/src/config/Language.ts";
68+
69+
function log(id, msg, isOk) {
70+
const el = document.getElementById(id);
71+
const ts = new Date().toISOString().slice(11, 23);
72+
el.textContent = `[${ts}] ${msg}`;
73+
el.className = isOk === true ? "ok" : isOk === false ? "fail" : "";
74+
}
75+
76+
function addPicker(containerId, attrs = {}) {
77+
const c = document.getElementById(containerId);
78+
c.innerHTML = "";
79+
const dp = document.createElement("ui5-date-picker");
80+
dp.setAttribute("format-pattern", "MMM d, y");
81+
if (attrs.value) dp.setAttribute("value", attrs.value);
82+
c.appendChild(dp);
83+
return dp;
84+
}
85+
86+
function removePicker(containerId) {
87+
document.getElementById(containerId).innerHTML = "";
88+
}
89+
90+
// ── Scenario 1: race ──────────────────────────────────────────────────────
91+
92+
document.getElementById("btn-race").addEventListener("click", () => {
93+
log("log1", "setLanguage('bg') called (no await) → immediately adding picker...");
94+
setLanguage("bg"); // intentionally NOT awaited
95+
try {
96+
addPicker("container1", { value: "фев 6, 2019" });
97+
log("log1", "Picker added. Open it to check first-day-of-week (should be Monday for bg).");
98+
} catch (e) {
99+
log("log1", `CRASH: ${e}`, false);
100+
}
101+
});
102+
103+
document.getElementById("btn-safe").addEventListener("click", async () => {
104+
log("log1", "awaiting setLanguage('bg')...");
105+
try {
106+
await setLanguage("bg");
107+
log("log1", "setLanguage resolved — adding picker");
108+
addPicker("container1", { value: "фев 6, 2019" });
109+
log("log1", "Picker added safely. Open it — Monday should be first day of week.", true);
110+
} catch (e) {
111+
log("log1", `Error: ${e}`, false);
112+
}
113+
});
114+
115+
document.getElementById("btn-reset").addEventListener("click", async () => {
116+
removePicker("container1");
117+
await setLanguage("en");
118+
log("log1", "Reset: picker removed, language set back to EN.");
119+
});
120+
121+
// ── Scenario 2: safe mount after await ────────────────────────────────────
122+
123+
document.getElementById("btn-scenario2").addEventListener("click", async () => {
124+
log("log2", "awaiting setLanguage('bg')...");
125+
await setLanguage("bg");
126+
log("log2", "Language is bg — mounting picker...");
127+
const dp = addPicker("container2", { value: "фев 6, 2019" });
128+
document.getElementById("btn-open2").onclick = () => {
129+
dp.open = true;
130+
};
131+
log("log2", "Done. Click 'Open picker' — first day of week should be Monday (bg locale).", true);
132+
});
133+
134+
// ── Scenario 3: rapid switches ────────────────────────────────────────────
135+
136+
document.getElementById("btn-rapid").addEventListener("click", async () => {
137+
log("log3", "Rapid: en → de → bg → en → bg (no await on each)...");
138+
setLanguage("en");
139+
setLanguage("de");
140+
setLanguage("bg");
141+
setLanguage("en");
142+
setLanguage("bg"); // last one wins — if CLDR races, we may get a poisoned cache
143+
// now mount immediately
144+
try {
145+
addPicker("container3", { value: "фев 6, 2019" });
146+
log("log3", "Picker mounted after rapid switches. Open it to check locale.");
147+
} catch (e) {
148+
log("log3", `CRASH on mount: ${e}`, false);
149+
}
150+
// wait a bit then check if re-render happened correctly
151+
setTimeout(() => {
152+
log("log3", "500ms later — if picker is open and shows Monday-first, the fix works.", true);
153+
}, 500);
154+
});
155+
156+
// capture unhandled promise rejections so crashes show in the log
157+
window.addEventListener("unhandledrejection", (e) => {
158+
console.error("[unhandledrejection]", e.reason);
159+
});
160+
</script>
161+
</body>
162+
</html>

0 commit comments

Comments
 (0)