Skip to content

Commit 6713291

Browse files
t3dotggclaude
t3dotgg
and
claude
committed
fix: preserve legacy sidebar v2 opt-in and gate on hydration
Review caught two real regressions in the per-channel default. Legacy opt-in was silently discarded: users who had turned sidebar v2 on before `sidebarV2ConfiguredByUser` existed have `sidebarV2Enabled: true` with no companion flag, so they fell back to the channel default and lost the beta on production. `true` was never the schema default, so it can only come from the toggle — treat it as an explicit choice, matching how `normalizeDesktopSettingsDocument` reads a legacy stored `updateChannel`. Both platforms also resolved against pre-load state, mounting one sidebar and swapping it a tick later. Web now gates on client-settings hydration and mobile on the preferences atom having loaded; both hold v1 until then. The web flash predates this PR — the pre-hydration snapshot was always `false`, so opted-in users already saw a v1→v2 flip. Resolution moves into `resolveSidebarV2Enabled` alongside the mobile counterpart, so both are covered by pure tests. Also adds patch-decoder coverage for the field the beta toggle actually writes. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
1 parent 08d8f88 commit 6713291

7 files changed

Lines changed: 162 additions & 12 deletions

File tree

‎apps/mobile/src/features/threads/threadListV2.test.ts‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,42 @@ const NOW = "2026-06-02T00:00:00.000Z";
4141

4242
describe("resolveThreadListV2Enabled", () => {
4343
it.each(["development", "preview"])("defaults on for the %s variant", (appVariant) => {
44-
expect(resolveThreadListV2Enabled({ preference: undefined, appVariant })).toBe(true);
44+
expect(
45+
resolveThreadListV2Enabled({ preference: undefined, preferencesLoaded: true, appVariant }),
46+
).toBe(true);
4547
});
4648

4749
it.each(["production", undefined])("defaults off for the %s variant", (appVariant) => {
48-
expect(resolveThreadListV2Enabled({ preference: undefined, appVariant })).toBe(false);
50+
expect(
51+
resolveThreadListV2Enabled({ preference: undefined, preferencesLoaded: true, appVariant }),
52+
).toBe(false);
4953
});
5054

5155
it("prefers an explicit device choice over the variant default", () => {
52-
expect(resolveThreadListV2Enabled({ preference: false, appVariant: "preview" })).toBe(false);
53-
expect(resolveThreadListV2Enabled({ preference: true, appVariant: "production" })).toBe(true);
56+
expect(
57+
resolveThreadListV2Enabled({
58+
preference: false,
59+
preferencesLoaded: true,
60+
appVariant: "preview",
61+
}),
62+
).toBe(false);
63+
expect(
64+
resolveThreadListV2Enabled({
65+
preference: true,
66+
preferencesLoaded: true,
67+
appVariant: "production",
68+
}),
69+
).toBe(true);
70+
});
71+
72+
it("holds v1 while preferences are still loading so the list does not remount", () => {
73+
expect(
74+
resolveThreadListV2Enabled({
75+
preference: undefined,
76+
preferencesLoaded: false,
77+
appVariant: "development",
78+
}),
79+
).toBe(false);
5480
});
5581
});
5682

‎apps/mobile/src/features/threads/threadListV2.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,21 @@ export function resolveThreadListV2Default(appVariant: unknown): boolean {
3131
* Resolved Thread List v2 state: the device-local preference if the user has
3232
* set one, otherwise the default for this app variant. Preferences persist as
3333
* sparse patches, so `undefined` genuinely means "never chosen".
34+
*
35+
* `preferencesLoaded` guards the startup window: preferences load
36+
* asynchronously, and treating "still loading" as "never chosen" would mount
37+
* v2 on a development build and then flip to v1 once a stored opt-out arrives,
38+
* remounting the whole list. While loading, hold v1 — the state both variants
39+
* already start from.
3440
*/
3541
export function resolveThreadListV2Enabled(input: {
3642
readonly preference: boolean | undefined;
43+
readonly preferencesLoaded: boolean;
3744
readonly appVariant: unknown;
3845
}): boolean {
46+
if (!input.preferencesLoaded) {
47+
return false;
48+
}
3949
return input.preference ?? resolveThreadListV2Default(input.appVariant);
4050
}
4151

‎apps/mobile/src/features/threads/use-thread-list-v2-enabled.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ import { resolveThreadListV2Enabled } from "./threadListV2";
1616
*/
1717
export function useThreadListV2Enabled(): boolean {
1818
const preferencesResult = useAtomValue(mobilePreferencesAtom);
19+
const loaded = AsyncResult.isSuccess(preferencesResult);
1920
return resolveThreadListV2Enabled({
20-
preference: AsyncResult.isSuccess(preferencesResult)
21-
? preferencesResult.value.threadListV2Enabled
22-
: undefined,
21+
preference: loaded ? preferencesResult.value.threadListV2Enabled : undefined,
22+
preferencesLoaded: loaded,
2323
appVariant: Constants.expoConfig?.extra?.appVariant,
2424
});
2525
}

‎apps/web/src/branding.logic.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,38 @@ export function resolveSidebarV2Default(stageLabel: string): boolean {
2424
return stage === "nightly" || stage === "dev";
2525
}
2626

27+
/**
28+
* Resolved sidebar v2 state: an explicit choice if the user has made one,
29+
* otherwise the default for this build stage.
30+
*
31+
* A stored `enabled: true` counts as an explicit choice even without the
32+
* companion flag. `true` was never the schema default, so it can only have come
33+
* from the Settings → Beta toggle — settings written before that flag existed
34+
* would otherwise lose the opt-in and drop such users back to v1 on production.
35+
* Mirrors how `normalizeDesktopSettingsDocument` treats a legacy stored
36+
* `updateChannel: "nightly"` as user-configured.
37+
*
38+
* `settingsHydrated` guards the startup window: client settings load
39+
* asynchronously and the pre-hydration snapshot is just the schema defaults, so
40+
* resolving against it would mount one sidebar and swap it out a tick later,
41+
* remounting the tree. While hydrating, hold v1 — where both paths already
42+
* start.
43+
*/
44+
export function resolveSidebarV2Enabled(input: {
45+
readonly enabled: boolean;
46+
readonly configuredByUser: boolean;
47+
readonly settingsHydrated: boolean;
48+
readonly stageLabel: string;
49+
}): boolean {
50+
if (!input.settingsHydrated) {
51+
return false;
52+
}
53+
54+
return input.configuredByUser || input.enabled
55+
? input.enabled
56+
: resolveSidebarV2Default(input.stageLabel);
57+
}
58+
2759
export function resolveServerBackedAppStageLabel(input: {
2860
readonly primaryServerVersion: string | null | undefined;
2961
readonly fallbackStageLabel: string;

‎apps/web/src/branding.test.ts‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
resolveServerBackedAppDisplayName,
44
resolveServerBackedAppStageLabel,
55
resolveSidebarV2Default,
6+
resolveSidebarV2Enabled,
67
} from "./branding.logic";
78

89
const originalWindow = globalThis.window;
@@ -125,3 +126,64 @@ describe("resolveSidebarV2Default", () => {
125126
expect(resolveSidebarV2Default(stage)).toBe(false);
126127
});
127128
});
129+
130+
describe("resolveSidebarV2Enabled", () => {
131+
const hydrated = { settingsHydrated: true } as const;
132+
133+
it.each(["Alpha", "Latest"])(
134+
"keeps a legacy opt-in on %s builds even without the companion flag",
135+
(stageLabel) => {
136+
// `true` was never the schema default, so it can only be an explicit
137+
// opt-in from settings written before `sidebarV2ConfiguredByUser` existed.
138+
expect(
139+
resolveSidebarV2Enabled({
140+
...hydrated,
141+
enabled: true,
142+
configuredByUser: false,
143+
stageLabel,
144+
}),
145+
).toBe(true);
146+
},
147+
);
148+
149+
it("applies the stage default when the beta was never enabled or configured", () => {
150+
expect(
151+
resolveSidebarV2Enabled({
152+
...hydrated,
153+
enabled: false,
154+
configuredByUser: false,
155+
stageLabel: "Nightly",
156+
}),
157+
).toBe(true);
158+
expect(
159+
resolveSidebarV2Enabled({
160+
...hydrated,
161+
enabled: false,
162+
configuredByUser: false,
163+
stageLabel: "Latest",
164+
}),
165+
).toBe(false);
166+
});
167+
168+
it("honors an explicit opt-out over the stage default", () => {
169+
expect(
170+
resolveSidebarV2Enabled({
171+
...hydrated,
172+
enabled: false,
173+
configuredByUser: true,
174+
stageLabel: "Nightly",
175+
}),
176+
).toBe(false);
177+
});
178+
179+
it("holds v1 until settings hydrate so the sidebar does not remount", () => {
180+
expect(
181+
resolveSidebarV2Enabled({
182+
enabled: true,
183+
configuredByUser: true,
184+
settingsHydrated: false,
185+
stageLabel: "Nightly",
186+
}),
187+
).toBe(false);
188+
});
189+
});

‎apps/web/src/hooks/useSettings.ts‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import {
2525
} from "@t3tools/contracts/settings";
2626
import { safeErrorLogAttributes } from "@t3tools/client-runtime/errors";
2727
import { APP_STAGE_LABEL } from "~/branding";
28-
import { resolveSidebarV2Default } from "~/branding.logic";
28+
import { resolveSidebarV2Enabled } from "~/branding.logic";
2929
import { ensureLocalApi } from "~/localApi";
3030
import * as Struct from "effect/Struct";
3131
import { primaryServerSettingsAtom, serverEnvironment } from "~/state/server";
@@ -226,12 +226,23 @@ export function useClientSettings<T = ClientSettings>(
226226
* dev, off for production). Every consumer must read through this rather than
227227
* `settings.sidebarV2Enabled`, which is only meaningful alongside
228228
* `sidebarV2ConfiguredByUser`.
229+
*
230+
* Held at v1 until client settings hydrate. The pre-hydration snapshot is just
231+
* the schema defaults, so resolving against it would mount one sidebar and then
232+
* swap it out once persisted settings land — remounting the whole tree.
229233
*/
230234
export function useSidebarV2Enabled(): boolean {
231-
return useClientSettings((settings) =>
232-
settings.sidebarV2ConfiguredByUser
233-
? settings.sidebarV2Enabled
234-
: resolveSidebarV2Default(APP_STAGE_LABEL),
235+
const settingsHydrated = useClientSettingsHydrated();
236+
const settings = useClientSettingsValue();
237+
return useMemo(
238+
() =>
239+
resolveSidebarV2Enabled({
240+
enabled: settings.sidebarV2Enabled,
241+
configuredByUser: settings.sidebarV2ConfiguredByUser,
242+
settingsHydrated,
243+
stageLabel: APP_STAGE_LABEL,
244+
}),
245+
[settings.sidebarV2Enabled, settings.sidebarV2ConfiguredByUser, settingsHydrated],
235246
);
236247
}
237248

‎packages/contracts/src/settings.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,15 @@ describe("ClientSettings sidebar v2", () => {
7272
expect(settings.sidebarV2ConfiguredByUser).toBe(true);
7373
});
7474

75+
it("carries an explicit beta opt-out through the patch the beta toggle writes", () => {
76+
const patch = decodeClientSettingsPatch({
77+
sidebarV2Enabled: false,
78+
sidebarV2ConfiguredByUser: true,
79+
});
80+
expect(patch.sidebarV2Enabled).toBe(false);
81+
expect(patch.sidebarV2ConfiguredByUser).toBe(true);
82+
});
83+
7584
it("allows auto-settle by inactivity to be disabled", () => {
7685
expect(
7786
decodeClientSettings({ sidebarAutoSettleAfterDays: null }).sidebarAutoSettleAfterDays,

0 commit comments

Comments
 (0)