-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix(desktop): show cached display names on startup #3317
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,10 @@ import { | |
| shouldFetchAvatar, | ||
| resolveAvatarDataUrl, | ||
| } from "@/features/profile/lib/selfProfileStorage"; | ||
| import { | ||
| readCachedUserLabels, | ||
| writeCachedUserLabels, | ||
| } from "@/features/profile/lib/userLabelStorage"; | ||
| import { useCommunities } from "@/features/communities/useCommunities"; | ||
| import { updateCachedChannelMemberDisplayName } from "@/features/channels/channelMemberProfileCache"; | ||
|
|
||
|
|
@@ -317,6 +321,8 @@ export function useUsersBatchQuery( | |
| }, | ||
| ) { | ||
| const queryClient = useQueryClient(); | ||
| const { activeCommunity } = useCommunities(); | ||
| const relayUrl = activeCommunity?.relayUrl ?? ""; | ||
| const normalizedPubkeys = [ | ||
| ...new Set(pubkeys.map((pubkey) => pubkey.toLowerCase())), | ||
| ] | ||
|
|
@@ -352,6 +358,9 @@ export function useUsersBatchQuery( | |
| } | ||
| if (toFetch.length > 0) { | ||
| const fresh = await getUsersBatch(toFetch); | ||
| if (relayUrl) { | ||
| writeCachedUserLabels(relayUrl, fresh.profiles, fresh.missing); | ||
| } | ||
| for (const pubkey of toFetch) { | ||
| const summary = fresh.profiles[pubkey] ?? null; | ||
| queryClient.setQueryData<UsersBatchEntry>( | ||
|
|
@@ -368,13 +377,20 @@ export function useUsersBatchQuery( | |
| // key entirely. Without this, already-resolved authors would flash back | ||
| // to their raw pubkey while the larger batch refetches. | ||
| placeholderData: keepPreviousData, | ||
| initialData: relayUrl | ||
| ? () => readCachedUserLabels(relayUrl, normalizedPubkeys) | ||
| : undefined, | ||
| initialDataUpdatedAt: 0, | ||
|
Comment on lines
+380
to
+383
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 I think
Suggested fix — deliver the cache through the placeholder chain instead: placeholderData: (prev) => prev ?? readCachedUserLabels(relayUrl, normalizedPubkeys),and drop |
||
| staleTime: 60_000, | ||
| gcTime: 5 * 60 * 1_000, | ||
| }); | ||
|
|
||
| // Seed individual "user-profile" cache entries so avatar clicks are instant | ||
| // cache hits instead of fresh network requests. | ||
| React.useEffect(() => { | ||
| // Persisted labels are intentionally presentation-only. Wait for a relay | ||
| // result before seeding profile-detail caches that also carry ownership. | ||
| if (query.dataUpdatedAt === 0) return; | ||
| const profiles = query.data?.profiles; | ||
| if (!profiles) return; | ||
| for (const [pubkey, summary] of Object.entries(profiles)) { | ||
|
|
@@ -391,7 +407,7 @@ export function useUsersBatchQuery( | |
| }, | ||
| ); | ||
| } | ||
| }, [query.data, queryClient]); | ||
| }, [query.data, query.dataUpdatedAt, queryClient]); | ||
|
|
||
| return query; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,185 @@ | ||
| import assert from "node:assert/strict"; | ||
| import test from "node:test"; | ||
|
|
||
| async function loadSubject() { | ||
| try { | ||
| return await import("./userLabelStorage.ts"); | ||
| } catch { | ||
| return {}; | ||
| } | ||
| } | ||
|
|
||
| function installLocalStorage() { | ||
| const values = new Map(); | ||
| globalThis.window = { | ||
| localStorage: { | ||
| getItem: (key) => values.get(key) ?? null, | ||
| setItem: (key, value) => values.set(key, value), | ||
| removeItem: (key) => values.delete(key), | ||
| key: (index) => [...values.keys()][index] ?? null, | ||
| get length() { | ||
| return values.size; | ||
| }, | ||
| }, | ||
| }; | ||
| globalThis.localStorage = globalThis.window.localStorage; | ||
| return values; | ||
| } | ||
|
|
||
| test("reads cached labels as safe stale profile summaries", async () => { | ||
| const subject = await loadSubject(); | ||
| assert.equal(typeof subject.readCachedUserLabels, "function"); | ||
| installLocalStorage(); | ||
| window.localStorage.setItem( | ||
| "buzz-user-labels.v1:wss://relay.example", | ||
| JSON.stringify({ | ||
| version: 1, | ||
| updatedAt: 100, | ||
| profiles: { | ||
| abcdef: { | ||
| displayName: "Alice", | ||
| name: "alice", | ||
| nip05Handle: "[email protected]", | ||
| updatedAt: 100, | ||
| }, | ||
| }, | ||
| }), | ||
| ); | ||
|
|
||
| assert.deepEqual( | ||
| subject.readCachedUserLabels("WSS://Relay.Example/", ["ABCDEF", "missing"]), | ||
| { | ||
| profiles: { | ||
| abcdef: { | ||
| displayName: "Alice", | ||
| name: "alice", | ||
| avatarUrl: null, | ||
| nip05Handle: "[email protected]", | ||
| ownerPubkey: null, | ||
| }, | ||
| }, | ||
| missing: [], | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| test("writes merge with existing labels and remain bounded", async () => { | ||
| const subject = await loadSubject(); | ||
| assert.equal(typeof subject.writeCachedUserLabels, "function"); | ||
| installLocalStorage(); | ||
|
|
||
| subject.writeCachedUserLabels("wss://relay.example", { | ||
| existing: { | ||
| displayName: "Existing", | ||
| name: null, | ||
| avatarUrl: null, | ||
| nip05Handle: null, | ||
| ownerPubkey: null, | ||
| }, | ||
| }); | ||
| subject.writeCachedUserLabels( | ||
| "wss://relay.example", | ||
| Object.fromEntries( | ||
| Array.from({ length: 1_005 }, (_, index) => [ | ||
| `pubkey-${index}`, | ||
| { | ||
| displayName: `Person ${index}`, | ||
| name: null, | ||
| avatarUrl: null, | ||
| nip05Handle: null, | ||
| ownerPubkey: null, | ||
| }, | ||
| ]), | ||
| ), | ||
| ); | ||
|
|
||
| const stored = JSON.parse( | ||
| window.localStorage.getItem( | ||
| subject.userLabelCacheKey("wss://relay.example"), | ||
| ), | ||
| ); | ||
| assert.equal(Object.keys(stored.profiles).length, 1_000); | ||
| assert.equal(stored.version, 1); | ||
| }); | ||
|
|
||
| test("removes a stale label when the fresh profile clears all names", async () => { | ||
| const subject = await loadSubject(); | ||
| assert.equal(typeof subject.writeCachedUserLabels, "function"); | ||
| installLocalStorage(); | ||
|
|
||
| subject.writeCachedUserLabels("wss://relay.example", { | ||
| abcdef: { | ||
| displayName: "Alice", | ||
| name: "alice", | ||
| avatarUrl: null, | ||
| nip05Handle: null, | ||
| ownerPubkey: null, | ||
| }, | ||
| }); | ||
| subject.writeCachedUserLabels("wss://relay.example", { | ||
| abcdef: { | ||
| displayName: null, | ||
| name: null, | ||
| avatarUrl: null, | ||
| nip05Handle: null, | ||
| ownerPubkey: null, | ||
| }, | ||
| }); | ||
|
|
||
| assert.equal( | ||
| subject.readCachedUserLabels("wss://relay.example", ["abcdef"]), | ||
| undefined, | ||
| ); | ||
| }); | ||
|
|
||
| test("removes stale labels for profiles the relay reports missing", async () => { | ||
| const subject = await loadSubject(); | ||
| assert.equal(typeof subject.writeCachedUserLabels, "function"); | ||
| installLocalStorage(); | ||
|
|
||
| subject.writeCachedUserLabels("wss://relay.example", { | ||
| abcdef: { | ||
| displayName: "Alice", | ||
| name: "alice", | ||
| avatarUrl: null, | ||
| nip05Handle: null, | ||
| ownerPubkey: null, | ||
| }, | ||
| }); | ||
| subject.writeCachedUserLabels("wss://relay.example", {}, ["ABCDEF"]); | ||
|
|
||
| assert.equal( | ||
| subject.readCachedUserLabels("wss://relay.example", ["abcdef"]), | ||
| undefined, | ||
| ); | ||
| }); | ||
|
|
||
| test("removes only the selected relay cache", async () => { | ||
| const subject = await loadSubject(); | ||
| assert.equal(typeof subject.removeUserLabelCacheForRelay, "function"); | ||
| installLocalStorage(); | ||
| const first = subject.userLabelCacheKey("wss://one.example"); | ||
| const second = subject.userLabelCacheKey("wss://two.example"); | ||
| window.localStorage.setItem(first, "{}"); | ||
| window.localStorage.setItem(second, "{}"); | ||
|
|
||
| subject.removeUserLabelCacheForRelay("wss://one.example"); | ||
|
|
||
| assert.equal(window.localStorage.getItem(first), null); | ||
| assert.equal(window.localStorage.getItem(second), "{}"); | ||
| }); | ||
|
|
||
| test("ignores malformed cache payloads", async () => { | ||
| const subject = await loadSubject(); | ||
| assert.equal(typeof subject.readCachedUserLabels, "function"); | ||
| installLocalStorage(); | ||
| window.localStorage.setItem( | ||
| "buzz-user-labels.v1:wss://relay.example", | ||
| JSON.stringify({ version: 1, profiles: { abc: { displayName: 42 } } }), | ||
| ); | ||
|
|
||
| assert.equal( | ||
| subject.readCachedUserLabels("wss://relay.example", ["abc"]), | ||
| undefined, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 nit: this writes labels under the
relayUrlcaptured at render, so a fetch resolving across a community switch could in theory file profiles under the old relay's cache key. In practice thecommunityKeyremount tears the query down first, so I think this is theoretical — just noting it.