Skip to content

fix: Improve tab sync performance #2699

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

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

fix: Improve tab sync performance #2699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ac6214a
only share auth if present
SuaYoo Jun 27, 2025
bf3a4ba
only check freshness if visible
SuaYoo Sep 16, 2025
4090669
switch to instance
SuaYoo Sep 16, 2025
da64524
close broadcast channel when finished
SuaYoo Sep 17, 2025
672a5e7
handle log out
SuaYoo Sep 17, 2025
cb320a7
remove receipt logic
SuaYoo Sep 17, 2025
f56dc23
update tests
SuaYoo Sep 17, 2025
ffe5ffc
store logged in state in local storage
SuaYoo Sep 17, 2025
138e1a9
fix tests
SuaYoo Sep 17, 2025
3604b9e
aways save login during init
SuaYoo Sep 17, 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
63 changes: 18 additions & 45 deletions frontend/src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { expect, fixture } from "@open-wc/testing";
import { html } from "lit";
import { restore, stub } from "sinon";

import { APIController } from "./controllers/api";
import { NavigateController } from "./controllers/navigate";
import { NotifyController } from "./controllers/notify";
import { type AppSettings } from "./utils/app";
Expand Down Expand Up @@ -46,20 +47,20 @@ const mockAppSettings: AppSettings = {
salesEmail: "",
supportEmail: "",
localesEnabled: ["en", "es"],
numBrowsersPerInstance: 1,
};

describe("browsertrix-app", () => {
beforeEach(() => {
AppStateService.resetAll();
AuthService.broadcastChannel = new BroadcastChannel(AuthService.storageKey);
window.sessionStorage.clear();
window.localStorage.clear();
stub(window.history, "pushState");
stub(NotifyController.prototype, "toast");
stub(APIController.prototype, "fetch");
});

afterEach(() => {
AuthService.broadcastChannel.close();
restore();
});

Expand All @@ -69,7 +70,7 @@ describe("browsertrix-app", () => {
});

it("don't block render if settings aren't defined", async () => {
stub(AuthService, "initSessionStorage").returns(
stub(AuthService.prototype, "initSessionStorage").returns(
Promise.resolve({
headers: { Authorization: "_fake_headers_" },
tokenExpiresAt: 0,
Expand All @@ -85,16 +86,13 @@ describe("browsertrix-app", () => {
});

it("renders 404 when not in org", async () => {
stub(AuthService, "initSessionStorage").returns(
Promise.resolve({
headers: { Authorization: "_fake_headers_" },
tokenExpiresAt: 0,
username: "[email protected]",
}),
stub(AuthService.prototype, "initSessionStorage").returns(
Promise.resolve(mockAuth),
);
// @ts-expect-error checkFreshness is private
stub(AuthService.prototype, "checkFreshness");

AppStateService.updateAuth(mockAuth);
AppStateService.updateUser(
formatAPIUser({
...mockAPIUser,
Expand All @@ -117,16 +115,13 @@ describe("browsertrix-app", () => {
role: 10,
};

stub(AuthService, "initSessionStorage").returns(
Promise.resolve({
headers: { Authorization: "_fake_headers_" },
tokenExpiresAt: 0,
username: "[email protected]",
}),
stub(AuthService.prototype, "initSessionStorage").returns(
Promise.resolve(mockAuth),
);
// @ts-expect-error checkFreshness is private
stub(AuthService.prototype, "checkFreshness");

AppStateService.updateAuth(mockAuth);
AppStateService.updateUser(
formatAPIUser({
...mockAPIUser,
Expand All @@ -142,7 +137,9 @@ describe("browsertrix-app", () => {
});

it("renders log in when not authenticated", async () => {
stub(AuthService, "initSessionStorage").returns(Promise.resolve(null));
stub(AuthService.prototype, "initSessionStorage").returns(
Promise.resolve(null),
);
// @ts-expect-error checkFreshness is private
stub(AuthService.prototype, "checkFreshness");
stub(NavigateController, "createNavigateEvent").callsFake(
Expand All @@ -158,33 +155,12 @@ describe("browsertrix-app", () => {
expect(el.shadowRoot?.querySelector("btrix-log-in")).to.exist;
});

// TODO move tests to AuthService
it("sets auth state from session storage", async () => {
stub(AuthService.prototype, "startFreshnessCheck").callsFake(() => {});
stub(window.sessionStorage, "getItem").callsFake((key) => {
if (key === "btrix.auth")
return JSON.stringify({
headers: { Authorization: "_fake_headers_" },
tokenExpiresAt: 0,
username: "[email protected]",
});
return null;
});
const el = await fixture<App>("<browsertrix-app></browsertrix-app>");

expect(el.authService.authState).to.eql({
headers: { Authorization: "_fake_headers_" },
tokenExpiresAt: 0,
username: "[email protected]",
});
});

it("sets user info", async () => {
stub(App.prototype, "getUserInfo").callsFake(async () =>
Promise.resolve(mockAPIUser),
);
stub(AuthService.prototype, "startFreshnessCheck").callsFake(() => {});
stub(AuthService, "initSessionStorage").callsFake(async () =>
stub(AuthService.prototype, "initSessionStorage").callsFake(async () =>
Promise.resolve({
headers: { Authorization: "_fake_headers_" },
tokenExpiresAt: 0,
Expand All @@ -203,7 +179,7 @@ describe("browsertrix-app", () => {
Promise.resolve(mockAPIUser),
);
stub(AuthService.prototype, "startFreshnessCheck").callsFake(() => {});
stub(AuthService, "initSessionStorage").callsFake(async () =>
stub(AuthService.prototype, "initSessionStorage").callsFake(async () =>
Promise.resolve({
headers: { Authorization: "_fake_headers_" },
tokenExpiresAt: 0,
Expand All @@ -227,6 +203,7 @@ describe("browsertrix-app", () => {
slug: id,
role: 10,
};
AppStateService.updateAuth(mockAuth);
AppStateService.updateUser(
formatAPIUser({
...mockAPIUser,
Expand All @@ -235,12 +212,8 @@ describe("browsertrix-app", () => {
);
stub(App.prototype, "getLocationPathname").callsFake(() => `/orgs/${id}`);
stub(AuthService.prototype, "startFreshnessCheck").callsFake(() => {});
stub(AuthService, "initSessionStorage").callsFake(async () =>
Promise.resolve({
headers: { Authorization: "_fake_headers_" },
tokenExpiresAt: 0,
username: "[email protected]",
}),
stub(AuthService.prototype, "initSessionStorage").callsFake(async () =>
Promise.resolve(mockAuth),
);

const el = await fixture<App>("<browsertrix-app></browsertrix-app>");
Expand Down
32 changes: 4 additions & 28 deletions frontend/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,13 @@ import type { UserInfo, UserOrg } from "./types/user";
import { pageView, type AnalyticsTrackProps } from "./utils/analytics";
import { type ViewState } from "./utils/APIRouter";
import AuthService, {
type AuthEventDetail,
type LoggedInEventDetail,
type NeedLoginEventDetail,
} from "./utils/AuthService";

import { BtrixElement } from "@/classes/BtrixElement";
import type { NavigateEventDetail } from "@/controllers/navigate";
import type { NotifyEventDetail } from "@/controllers/notify";
import { type Auth } from "@/types/auth";
import {
translatedLocales,
type TranslatedLocaleEnum,
Expand Down Expand Up @@ -139,14 +137,13 @@ export class App extends BtrixElement {
async connectedCallback() {
let authState: AuthService["authState"] = null;
try {
authState = await AuthService.initSessionStorage();
authState = await this.authService.initSessionStorage();
} catch (e) {
console.debug(e);
}

this.syncViewState();
if (authState) {
this.authService.saveLogin(authState);
}

if (authState && !this.userInfo) {
void this.fetchAndUpdateUserInfo();
}
Expand All @@ -162,8 +159,6 @@ export class App extends BtrixElement {
window.addEventListener("popstate", () => {
this.syncViewState();
});

this.startSyncBrowserTabs();
}

private attachUserGuideListeners() {
Expand Down Expand Up @@ -1085,6 +1080,7 @@ export class App extends BtrixElement {

private clearUser() {
this.authService.logout();
this.authService.finalize();
this.authService = new AuthService();
AppStateService.resetUser();
}
Expand Down Expand Up @@ -1130,26 +1126,6 @@ export class App extends BtrixElement {
});
}

private startSyncBrowserTabs() {
AuthService.broadcastChannel.addEventListener(
"message",
({ data }: { data: AuthEventDetail }) => {
if (data.name === "auth_storage") {
if (data.value !== AuthService.storage.getItem()) {
if (data.value) {
this.authService.saveLogin(JSON.parse(data.value) as Auth);
void this.fetchAndUpdateUserInfo();
this.syncViewState();
} else {
this.clearUser();
this.routeTo(urlForName("login"));
}
}
}
},
);
}

private clearSelectedOrg() {
AppStateService.updateOrgSlug(null);
}
Expand Down
70 changes: 40 additions & 30 deletions frontend/src/utils/AuthService.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,32 +4,41 @@ import { restore, stub } from "sinon";
import AuthService from "./AuthService";
import { AppStateService } from "./state";

import { APIController } from "@/controllers/api";

describe("AuthService", () => {
beforeEach(() => {
AppStateService.resetAll();
AuthService.broadcastChannel = new BroadcastChannel(AuthService.storageKey);
window.sessionStorage.clear();
window.sessionStorage.clear();
stub(window.history, "pushState");
stub(APIController.prototype, "fetch");
stub(AuthService.prototype, "refresh");
});

afterEach(() => {
AuthService.broadcastChannel.close();
restore();
});

describe("AuthService.initSessionStorage()", () => {
describe("initSessionStorage()", () => {
let authService = new AuthService();

beforeEach(() => {
authService = new AuthService();
});

it("returns auth in session storage", async () => {
stub(window.sessionStorage, "getItem").returns(
JSON.stringify({
headers: { Authorization: "_fake_headers_" },
tokenExpiresAt: "_fake_tokenExpiresAt_",
tokenExpiresAt: 1111,
username: "[email protected]",
}),
);
const result = await AuthService.initSessionStorage();
const result = await authService.initSessionStorage();
expect(result).to.deep.equal({
headers: { Authorization: "_fake_headers_" },
tokenExpiresAt: "_fake_tokenExpiresAt_",
tokenExpiresAt: 1111,
username: "[email protected]",
});
});
Expand All @@ -41,22 +50,43 @@ describe("AuthService", () => {
name: "responding_auth",
auth: {
headers: { Authorization: "_fake_headers_from_tab_" },
tokenExpiresAt: "_fake_tokenExpiresAt_from_tab_",
tokenExpiresAt: 9999,
username: "[email protected]_from_tab_",
},
});
});
const result = await AuthService.initSessionStorage();
const result = await authService.initSessionStorage();
expect(result).to.deep.equal({
headers: { Authorization: "_fake_headers_from_tab_" },
tokenExpiresAt: "_fake_tokenExpiresAt_from_tab_",
tokenExpiresAt: 9999,
username: "[email protected]_from_tab_",
});
otherTabChannel.close();
});
it("saves auth in session storage", async () => {
stub(window.sessionStorage, "getItem");
const otherTabChannel = new BroadcastChannel(AuthService.storageKey);
otherTabChannel.addEventListener("message", () => {
otherTabChannel.postMessage({
name: "responding_auth",
auth: {
headers: { Authorization: "_fake_headers_from_tab_to_save_" },
tokenExpiresAt: 9999,
username: "[email protected]_from_tab_to_save_",
},
});
});
await authService.initSessionStorage();
expect(authService.authState).to.deep.equal({
headers: { Authorization: "_fake_headers_from_tab_to_save_" },
tokenExpiresAt: 9999,
username: "[email protected]_from_tab_to_save_",
});
otherTabChannel.close();
});
it("resolves without stored auth or another tab", async () => {
stub(window.sessionStorage, "getItem");
const result = await AuthService.initSessionStorage();
const result = await authService.initSessionStorage();
expect(result).to.equal(null);
});
});
Expand Down Expand Up @@ -84,26 +114,6 @@ describe("AuthService", () => {
expect(window.sessionStorage.setItem).to.have.been.called;
});

it("posts to the broadcast channel", () => {
stub(AuthService.storage, "getItem").returns("");
stub(AuthService.broadcastChannel, "postMessage");
stub(window.sessionStorage, "setItem");

const authValue = JSON.stringify({
headers: { Authorization: self.crypto.randomUUID() },
tokenExpiresAt: Date.now(),
username: "[email protected]",
});
AuthService.storage.setItem(authValue);

expect(
AuthService.broadcastChannel.postMessage,
).to.have.been.calledWith({
name: "auth_storage",
value: authValue,
});
});

it("does not store the same value", () => {
stub(AuthService.storage, "getItem").returns(JSON.stringify(mockAuth));
stub(window.sessionStorage, "setItem");
Expand Down
Loading
Loading