Skip to content

Commit 9597cb1

Browse files
SuaYooSuaYoo
authored
chore: Prevent blocking connected callback (#2244)
Moves language initialization to `willUpdate` to prevent blocking connected callback and attaching listeners
1 parent 02eeaca commit 9597cb1

File tree

5 files changed

+61
-22
lines changed

5 files changed

+61
-22
lines changed

‎.vscode/snippets.code-snippets

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
"",
1010
"import { BtrixElement } from \"@/classes/BtrixElement\";",
1111
"",
12-
"@localized()",
1312
"@customElement(\"btrix-${1:component}\")",
13+
"@localized()",
1414
"export class ${2:Component} extends BtrixElement {",
1515
"\trender() {",
1616
"\t\treturn html``;",

‎frontend/src/index.ejs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
</head>
2323
<body>
2424
<script>
25+
// Fetch API settings in parallel with dynamically loaded components
2526
window
2627
.fetch("/api/settings", {
2728
headers: { "Content-Type": "application/json" },

‎frontend/src/index.test.ts

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { NavigateController } from "./controllers/navigate";
66
import { NotifyController } from "./controllers/notify";
77
import { type AppSettings } from "./utils/app";
88
import AuthService from "./utils/AuthService";
9-
import appState, { AppStateService } from "./utils/state";
9+
import { AppStateService } from "./utils/state";
1010
import { formatAPIUser } from "./utils/user";
1111

1212
import { App, type APIUser } from ".";
@@ -62,7 +62,7 @@ describe("browsertrix-app", () => {
6262
expect(el).instanceOf(App);
6363
});
6464

65-
it("renders home when authenticated", async () => {
65+
it("blocks render if settings aren't defined", async () => {
6666
stub(AuthService, "initSessionStorage").returns(
6767
Promise.resolve({
6868
headers: { Authorization: "_fake_headers_" },
@@ -72,25 +72,43 @@ describe("browsertrix-app", () => {
7272
);
7373
// @ts-expect-error checkFreshness is private
7474
stub(AuthService.prototype, "checkFreshness");
75-
stub(appState, "settings").returns(mockAppSettings);
7675
const el = await fixture<App>(html` <browsertrix-app></browsertrix-app>`);
7776
await el.updateComplete;
77+
78+
expect(el.shadowRoot?.childElementCount).to.equal(0);
79+
});
80+
81+
it("renders home when authenticated", async () => {
82+
stub(AuthService, "initSessionStorage").returns(
83+
Promise.resolve({
84+
headers: { Authorization: "_fake_headers_" },
85+
tokenExpiresAt: 0,
86+
username: "test-auth@example.com",
87+
}),
88+
);
89+
// @ts-expect-error checkFreshness is private
90+
stub(AuthService.prototype, "checkFreshness");
91+
const el = await fixture<App>(
92+
html` <browsertrix-app .settings=${mockAppSettings}></browsertrix-app>`,
93+
);
94+
await el.updateComplete;
7895
expect(el.shadowRoot?.querySelector("btrix-home")).to.exist;
7996
});
8097

8198
it("renders home when not authenticated", async () => {
8299
stub(AuthService, "initSessionStorage").returns(Promise.resolve(null));
83100
// @ts-expect-error checkFreshness is private
84101
stub(AuthService.prototype, "checkFreshness");
85-
stub(appState, "settings").returns(mockAppSettings);
86102
stub(NavigateController, "createNavigateEvent").callsFake(
87103
() =>
88104
new CustomEvent("x-ignored", {
89105
detail: { url: "", resetScroll: false },
90106
}),
91107
);
92108

93-
const el = await fixture<App>(html` <browsertrix-app></browsertrix-app>`);
109+
const el = await fixture<App>(
110+
html` <browsertrix-app .settings=${mockAppSettings}></browsertrix-app>`,
111+
);
94112
expect(el.shadowRoot?.querySelector("btrix-home")).to.exist;
95113
});
96114

‎frontend/src/index.ts

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ import {
4040
translatedLocales,
4141
type TranslatedLocaleEnum,
4242
} from "@/types/localization";
43-
import { getAppSettings, type AppSettings } from "@/utils/app";
43+
import { type AppSettings } from "@/utils/app";
4444
import { DEFAULT_MAX_SCALE } from "@/utils/crawler";
4545
import localize from "@/utils/localize";
4646
import { toast } from "@/utils/notify";
@@ -70,21 +70,33 @@ export interface UserGuideEventMap {
7070
"btrix-user-guide-show": CustomEvent<{ path?: string }>;
7171
}
7272

73-
@localized()
7473
@customElement("browsertrix-app")
74+
@localized()
7575
export class App extends BtrixElement {
76+
/**
77+
* Browsertrix app version to display in the UI
78+
*/
7679
@property({ type: String })
7780
version?: string;
7881

82+
/**
83+
* Base URL for user guide documentation
84+
*/
7985
@property({ type: String })
8086
docsUrl = "/docs/";
8187

88+
/**
89+
* App settings from `/api/settings`
90+
*/
8291
@property({ type: Object })
8392
settings?: AppSettings;
8493

8594
private readonly router = new APIRouter(ROUTES);
8695
authService = new AuthService();
8796

97+
@state()
98+
private translationReady = false;
99+
88100
@state()
89101
private viewState!: ViewState<typeof ROUTES>;
90102

@@ -116,19 +128,6 @@ export class App extends BtrixElement {
116128
void this.fetchAndUpdateUserInfo();
117129
}
118130

119-
try {
120-
this.settings = await getAppSettings();
121-
} catch (e) {
122-
console.error(e);
123-
this.notify.toast({
124-
message: msg("Couldn’t initialize Browsertrix correctly."),
125-
variant: "danger",
126-
icon: "exclamation-octagon",
127-
id: "get-app-settings-error",
128-
});
129-
} finally {
130-
await localize.initLanguage();
131-
}
132131
super.connectedCallback();
133132

134133
this.addEventListener("btrix-navigate", this.onNavigateTo);
@@ -157,6 +156,10 @@ export class App extends BtrixElement {
157156
willUpdate(changedProperties: Map<string, unknown>) {
158157
if (changedProperties.has("settings")) {
159158
AppStateService.updateSettings(this.settings || null);
159+
160+
if (this.settings && !changedProperties.get("settings")) {
161+
void this.initTranslation();
162+
}
160163
}
161164
if (changedProperties.has("viewState")) {
162165
if (this.viewState.route === "orgs") {
@@ -170,6 +173,13 @@ export class App extends BtrixElement {
170173
}
171174
}
172175

176+
async initTranslation() {
177+
await localize.initLanguage();
178+
// TODO We might want to set this in a lit-localize-status event listener
179+
// see https://lit.dev/docs/localization/runtime-mode/#example-of-using-the-status-event
180+
this.translationReady = true;
181+
}
182+
173183
getLocationPathname() {
174184
return window.location.pathname;
175185
}
@@ -264,6 +274,8 @@ export class App extends BtrixElement {
264274
}
265275

266276
render() {
277+
if (!this.translationReady) return;
278+
267279
return html`
268280
<div class="min-w-screen flex min-h-screen flex-col">
269281
${this.renderNavBar()} ${this.renderAlertBanner()}

‎frontend/src/utils/localize.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,19 @@ import {
2020
} from "@/types/localization";
2121
import appState from "@/utils/state";
2222

23+
// Pre-load all locales
24+
const localizedTemplates = new Map(
25+
targetLocales.map((locale) => [
26+
locale,
27+
import(`/src/__generated__/locales/${locale}.ts`),
28+
]),
29+
);
30+
2331
const { getLocale, setLocale } = configureLocalization({
2432
sourceLocale,
2533
targetLocales,
2634
loadLocale: async (locale: string) =>
27-
import(`/src/__generated__/locales/${locale}.ts`),
35+
localizedTemplates.get(locale as (typeof targetLocales)[number]),
2836
});
2937

3038
const defaultDateOptions: Intl.DateTimeFormatOptions = {

0 commit comments

Comments
 (0)