sorb/ThreadNet-Web
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 4 Wiki Activity

Update find-unused-settings script (#34252)

Browse Source 
* Move more tests over to vitest

* Add exception for test_setting

* Update find-unused-settings script

To not consider usage in tests as real usage

* Remove legacy font settings

They have been being migrated for over 2 years at this point and are flagged by the script as unused

* Re-add test
This commit is contained in:
Michael Telatynski
2026-07-15 10:21:49 +00:00
committed by GitHub
parent 703bd6177c
commit 04fac0a9a4
7 changed files with 17 additions and 204 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/web/src/dispatcher/actions.ts
-5
View File
@@ -108,11 +108,6 @@ export enum Action {
*/
ToggleSpacePanel = "toggle_space_panel",
/**
* Sets the apps root font size. Should be used with UpdateFontSizePayload
*/
MigrateBaseFontSize = "migrate_base_font_size",
/**
* Sets the apps root font size delta. Should be used with UpdateFontSizeDeltaPayload
* It will add the delta to the current font size.
apps/web/src/settings/Settings.tsx
-24
View File
@@ -238,8 +238,6 @@ export interface Settings {
"mjolnirPersonalRoom": IBaseSetting<string | null>;
"RoomList.backgroundImage": IBaseSetting<string | null>;
"sendReadReceipts": IBaseSetting<boolean>;
"baseFontSize": IBaseSetting<"" | number>;
"baseFontSizeV2": IBaseSetting<"" | number>;
"fontSizeDelta": IBaseSetting<number>;
"useCustomFontSize": IBaseSetting<boolean>;
"MessageComposerInput.suggestEmoji": IBaseSetting<boolean>;
@@ -623,15 +621,6 @@ export const SETTINGS: Settings = {
shouldWarn: true,
default: false,
},
/**
* @deprecated in favor of {@link fontSizeDelta}
*/
"baseFontSize": {
displayName: _td("settings|appearance|font_size"),
supportedLevels: LEVELS_ACCOUNT_SETTINGS,
default: "",
controller: new FontSizeController(),
},
"feature_render_reaction_images": {
isFeature: true,
labsGroup: LabGroup.Messaging,
@@ -658,21 +647,8 @@ export const SETTINGS: Settings = {
isFeature: true,
default: false,
},
/**
* With the transition to Compound we are moving to a base font size
* of 16px. We're taking the opportunity to move away from the `baseFontSize`
* setting that had a 5px offset.
* @deprecated in favor {@link fontSizeDelta}
*/
"baseFontSizeV2": {
displayName: _td("settings|appearance|font_size"),
supportedLevels: [SettingLevel.DEVICE],
default: "",
controller: new FontSizeController(),
},
/**
* This delta is added to the browser default font size
* Moving from `baseFontSizeV2` to `fontSizeDelta` to replace the default 16px to --cpd-font-size-root (browser default font size) + fontSizeDelta
*/
"fontSizeDelta": {
displayName: _td("settings|appearance|font_size"),
apps/web/src/settings/controllers/FontSizeController.test.ts
+2 -2
View File
@@ -13,7 +13,7 @@ import dis from "../../dispatcher/dispatcher";
import FontSizeController from "./FontSizeController";
import { SettingLevel } from "../SettingLevel";
const dispatchSpy = vi.spyOn(dis, "fire");
const dispatchSpy = vi.spyOn(dis, "dispatch");
describe("FontSizeController", () => {
it("dispatches a font size action on change", () => {
@@ -21,6 +21,6 @@ describe("FontSizeController", () => {
controller.onChange(SettingLevel.ACCOUNT, "$room:server", 12);
expect(dispatchSpy).toHaveBeenCalledWith(Action.MigrateBaseFontSize);
expect(dispatchSpy).toHaveBeenCalledWith({ action: Action.UpdateFontSizeDelta, delta: 12 });
});
});
apps/web/src/settings/controllers/FontSizeController.ts
+2 -8
View File
@@ -10,7 +10,7 @@ import SettingController from "./SettingController";
import dis from "../../dispatcher/dispatcher";
import { type UpdateFontSizeDeltaPayload } from "../../dispatcher/payloads/UpdateFontSizeDeltaPayload";
import { Action } from "../../dispatcher/actions";
import { SettingLevel } from "../SettingLevel";
import { type SettingLevel } from "../SettingLevel";
export default class FontSizeController extends SettingController {
public constructor() {
@@ -18,13 +18,7 @@ export default class FontSizeController extends SettingController {
}
public onChange(level: SettingLevel, roomId: string, newValue: any): void {
// In a distant past, `baseFontSize` was set on the account and config
// level. This can be accessed only after the initial sync. If we end up
// discovering that a logged in user has this kind of setting, we want to
// trigger another migration of the base font size.
if (level === SettingLevel.ACCOUNT || level === SettingLevel.CONFIG) {
dis.fire(Action.MigrateBaseFontSize);
} else if (newValue !== "") {
if (newValue !== "") {
// Dispatch font size change so that everything open responds to the change.
dis.dispatch<UpdateFontSizeDeltaPayload>({
action: Action.UpdateFontSizeDelta,
apps/web/src/settings/watchers/FontWatcher.ts
+1 -95
View File
@@ -11,7 +11,6 @@ import SettingsStore from "../SettingsStore";
import type IWatcher from "./Watcher";
import { toPx } from "../../utils/units";
import { Action } from "../../dispatcher/actions";
import { SettingLevel } from "../SettingLevel";
import { type UpdateSystemFontPayload } from "../../dispatcher/payloads/UpdateSystemFontPayload";
import { type ActionPayload } from "../../dispatcher/payloads";
@@ -33,97 +32,6 @@ export class FontWatcher implements IWatcher {
public async start(): Promise<void> {
this.updateFont();
this.dispatcherRef = dis.register(this.onAction);
/**
* baseFontSize is an account level setting which is loaded after the initial
* sync. Hence why we can't do that in the `constructor`
*/
await this.migrateBaseFontSize();
}
/**
* Migrate the base font size from the V1 and V2 version to the V3 version
* @private
*/
private async migrateBaseFontSize(): Promise<void> {
await this.migrateBaseFontV1toFontSizeDelta();
await this.migrateBaseFontV2toFontSizeDelta();
}
/**
* Migrating from the V1 version of the base font size to the new delta system.
* The delta system is using the default browser font size as a base
* Everything will become slightly larger, and getting rid of the `SIZE_DIFF`
* weirdness for locally persisted values
* @private
*/
private async migrateBaseFontV1toFontSizeDelta(): Promise<void> {
const legacyBaseFontSize = SettingsStore.getValue("baseFontSize");
// No baseFontV1 found, nothing to migrate
if (!legacyBaseFontSize) return;
console.log(
"Migrating base font size -> base font size V2 -> font size delta for Compound, current value",
legacyBaseFontSize,
);
// Compute the V1 to V2 version before migrating to fontSizeDelta
const baseFontSizeV2 = this.computeBaseFontSizeV1toV2(legacyBaseFontSize);
// Compute the difference between the V2 and the fontSizeDelta
const delta = this.computeFontSizeDeltaFromV2BaseFontSize(baseFontSizeV2);
await SettingsStore.setValue("fontSizeDelta", null, SettingLevel.DEVICE, delta);
await SettingsStore.setValue("baseFontSize", null, SettingLevel.DEVICE, 0);
console.log("Migration complete, deleting legacy `baseFontSize`");
}
/**
* Migrating from the V2 version of the base font size to the new delta system
* @private
*/
private async migrateBaseFontV2toFontSizeDelta(): Promise<void> {
const legacyBaseFontV2Size = SettingsStore.getValue("baseFontSizeV2");
// No baseFontV2 found, nothing to migrate
if (!legacyBaseFontV2Size) return;
console.log("Migrating base font size V2 for Compound, current value", legacyBaseFontV2Size);
// Compute the difference between the V2 and the fontSizeDelta
const delta = this.computeFontSizeDeltaFromV2BaseFontSize(legacyBaseFontV2Size);
await SettingsStore.setValue("fontSizeDelta", null, SettingLevel.DEVICE, delta);
await SettingsStore.setValue("baseFontSizeV2", null, SettingLevel.DEVICE, 0);
console.log("Migration complete, deleting legacy `baseFontSizeV2`");
}
/**
* Compute the V2 font size from the V1 font size
* @param legacyBaseFontSize
* @private
*/
private computeBaseFontSizeV1toV2(legacyBaseFontSize: number): number {
// For some odd reason, the persisted value in user storage has an offset
// of 5 pixels for all values stored under `baseFontSize`
const LEGACY_SIZE_DIFF = 5;
// Compound uses a base font size of `16px`, whereas the old Element
// styles based their calculations off a `15px` root font size.
const ROOT_FONT_SIZE_INCREASE = 1;
// Compute the font size of the V2 version before migrating to V3
return legacyBaseFontSize + ROOT_FONT_SIZE_INCREASE + LEGACY_SIZE_DIFF;
}
/**
* Compute the difference between the V2 font size and the default browser font size
* @param legacyBaseFontV2Size
* @private
*/
private computeFontSizeDeltaFromV2BaseFontSize(legacyBaseFontV2Size: number): number {
const browserDefaultFontSize = FontWatcher.getRootFontSize();
// Compute the difference between the V2 font size and the default browser font size
return legacyBaseFontV2Size - browserDefaultFontSize;
}
/**
@@ -157,9 +65,7 @@ export class FontWatcher implements IWatcher {
}
private onAction = (payload: ActionPayload): void => {
if (payload.action === Action.MigrateBaseFontSize) {
this.migrateBaseFontSize();
} else if (payload.action === Action.UpdateFontSizeDelta) {
if (payload.action === Action.UpdateFontSizeDelta) {
this.setRootFontSize(payload.delta);
} else if (payload.action === Action.UpdateSystemFont) {
this.setSystemFont(payload as UpdateSystemFontPayload);
apps/web/test/unit-tests/settings/watchers/FontWatcher-test.tsx
-60
View File
@@ -112,66 +112,6 @@ describe("FontWatcher", function () {
});
});
describe("Migrates baseFontSize", () => {
let watcher: FontWatcher | undefined;
beforeEach(() => {
document.documentElement.style.fontSize = "14px";
watcher = new FontWatcher();
});
afterEach(() => {
watcher!.stop();
});
it("should not run the migration", async () => {
await watcher!.start();
expect(SettingsStore.getValue("fontSizeDelta")).toBe(0);
});
it("should migrate from V1 font size to V3", async () => {
await SettingsStore.setValue("baseFontSize", null, SettingLevel.DEVICE, 13);
await watcher!.start();
// 13px (V1 font size) + 5px (V1 offset) + 1px (root font size increase) - 16px (default browser font size) = 3px
expect(SettingsStore.getValue("fontSizeDelta")).toBe(3);
// baseFontSize should be cleared
expect(SettingsStore.getValue("baseFontSize")).toBe(0);
});
it("should migrate from V2 font size to V3 using browser font size", async () => {
await SettingsStore.setValue("baseFontSizeV2", null, SettingLevel.DEVICE, 18);
await watcher!.start();
// 18px - 16px (default browser font size) = 2px
expect(SettingsStore.getValue("fontSizeDelta")).toBe(2);
// baseFontSize should be cleared
expect(SettingsStore.getValue("baseFontSizeV2")).toBe(0);
});
it("should migrate from V2 font size to V3 using fallback font size", async () => {
document.documentElement.style.fontSize = "";
await SettingsStore.setValue("baseFontSizeV2", null, SettingLevel.DEVICE, 18);
await watcher!.start();
// 18px - 16px (fallback) = 2px
expect(SettingsStore.getValue("fontSizeDelta")).toBe(2);
// baseFontSize should be cleared
expect(SettingsStore.getValue("baseFontSizeV2")).toBe(0);
});
it("should trigger migration when dispatched", async () => {
await watcher!.start();
await SettingsStore.setValue("baseFontSizeV2", null, SettingLevel.DEVICE, 18);
defaultDispatcher.fire(Action.MigrateBaseFontSize);
await waitFor(() => {
// 18px - 16px (default browser font size) = 2px
expect(SettingsStore.getValue("fontSizeDelta")).toBe(2);
// baseFontSizeV2 should be cleared
expect(SettingsStore.getValue("baseFontSizeV2")).toBe(0);
});
});
});
it("should update root font size with positive delta", async () => {
await new FontWatcher().start();
scripts/find-unused-settings.ts
Regular → Executable
+12 -10
View File
@@ -22,10 +22,15 @@ const ROOT = path.resolve(__dirname, "..");
const SETTINGS_DIR = path.join(ROOT, "apps/web/src/settings");
const SETTINGS_FILE = path.join(SETTINGS_DIR, "Settings.tsx");
// Only the settings *definitions* directory should be excluded from the usage search -
// there are plenty of other directories literally named "settings" (e.g.
// apps/web/src/components/views/settings/) which hold real usages and must stay included.
const SETTINGS_DIR_RELATIVE = path.relative(ROOT, SETTINGS_DIR);
const EXCLUDE_GLOBS = [
// Only the settings *definitions* directory should be excluded from the usage search -
// there are plenty of other directories literally named "settings" (e.g.
// apps/web/src/components/views/settings/) which hold real usages and must stay included.
"apps/web/src/settings/**",
"*/*/test/**",
"*-test.*",
"*.test.*",
].map((pattern) => path.relative(ROOT, pattern));
// See https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-error-message
const SETTINGS_FILE_RELATIVE = path.relative(ROOT, SETTINGS_FILE);
@@ -151,12 +156,9 @@ function findControllerText(settingValue: ts.Expression, sourceFile: ts.SourceFi
*/
function isUsedOutsideSettings(searchTerms: string[]): boolean {
const patternArgs = searchTerms.flatMap((term) => ["-e", term]);
const excludeArgs = EXCLUDE_GLOBS.map((term) => `:(exclude)${term}`);
try {
execFileSync(
"git",
["grep", "-F", "-q", ...patternArgs, "--", "apps", `:(exclude)${SETTINGS_DIR_RELATIVE}/**`],
{ cwd: ROOT },
);
execFileSync("git", ["grep", "-F", "-q", ...patternArgs, "--", "apps", ...excludeArgs], { cwd: ROOT });
return true;
} catch (e) {
if (typeof (e as { status?: number }).status === "number") return false;
@@ -205,7 +207,7 @@ function reportUnused(unused: DeclaredSetting[]): void {
for (const { name, line } of unused) {
console.error(` Settings.tsx:${line}: "${name}"`);
if (process.env.GITHUB_ACTIONS === "true") {
printAnnotation(line, `Setting "${name}" is declared but never used outside ${SETTINGS_DIR_RELATIVE}/`);
printAnnotation(line, `Setting "${name}" is declared but never used outside ${EXCLUDE_GLOBS}`);
}
}
}