Skip to content

Commit c3fcf6e

Browse files
authored
Merge pull request #763 from SolidOS/fix/friend-card-loading
fix friends freeze
2 parents 823c373 + 6239b49 commit c3fcf6e

2 files changed

Lines changed: 80 additions & 2 deletions

File tree

src/widgets/buttons.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,7 @@ export type attachmentListOptions = {
880880
noun?: string
881881
renderSupportingInfo?: RenderSupportingInfo
882882
renderNameSuffix?: RenderNameSuffix
883+
refreshOnDocumentLoad?: boolean
883884
}
884885

885886
/**
@@ -892,6 +893,11 @@ export function attachmentList (dom: HTMLDocument, subject: NamedNode, div: HTML
892893
// options = options || {}
893894
const docsWaitingForRowRefresh = new Set<string>()
894895
const hasAsyncEnrichedRowOptions = !!(options.renderSupportingInfo || options.renderNameSuffix)
896+
// Keep the generic default on for simple consumers: if row decoration depends on
897+
// linked-profile data arriving later, attachmentList will rerender once that
898+
// target document finishes loading. Complex callers with their own streaming or
899+
// batched refresh pipeline can opt out to avoid duplicate whole-list refreshes.
900+
const refreshOnDocumentLoad = options.refreshOnDocumentLoad ?? true
895901

896902
const deleteAttachment = function (target) {
897903
if (!kb.updater) {
@@ -918,13 +924,16 @@ export function attachmentList (dom: HTMLDocument, subject: NamedNode, div: HTML
918924
opt.renderSupportingInfo = options.renderSupportingInfo
919925
opt.renderNameSuffix = options.renderNameSuffix
920926

921-
if (hasAsyncEnrichedRowOptions && target?.uri && kb.fetcher) {
927+
if (hasAsyncEnrichedRowOptions && refreshOnDocumentLoad && target?.uri && kb.fetcher) {
922928
const targetDoc = target.doc()
923929
const requestState = targetDoc?.uri ? kb.fetcher.requested?.[targetDoc.uri] : undefined
924930
const shouldWaitForFetch = requestState !== 'done' && requestState !== 'failed'
925931
if (targetDoc?.uri && shouldWaitForFetch && !docsWaitingForRowRefresh.has(targetDoc.uri)) {
926932
docsWaitingForRowRefresh.add(targetDoc.uri)
927-
// Root fix: these row options can depend on async profile data, so rerender once fetch completes.
933+
// The row renderer may need data from the target profile that is not loaded yet.
934+
// Register one follow-up refresh per target document so simple attachmentList
935+
// consumers eventually show the enriched row contents without building their own
936+
// async refresh orchestration.
928937
kb.fetcher.nowOrWhenFetched(targetDoc, undefined, () => {
929938
docsWaitingForRowRefresh.delete(targetDoc.uri)
930939
refresh()

test/unit/widgets/buttons.test.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,11 @@ describe('askName', () => {
107107
})
108108

109109
describe('attachmentList', () => {
110+
afterEach(() => {
111+
clearStore()
112+
jest.restoreAllMocks()
113+
})
114+
110115
it('exists', () => {
111116
expect(attachmentList).toBeInstanceOf(Function)
112117
})
@@ -116,6 +121,70 @@ describe('attachmentList', () => {
116121
const options = {}
117122
expect(attachmentList(dom, subject, div, options)).toBeTruthy()
118123
})
124+
125+
it('refreshes rows after pending profile fetches by default', () => {
126+
const subject = sym('https://subject.example/profile/card#me')
127+
const predicate = ns.foaf('knows')
128+
const target = sym('https://friend.example/profile/card#me')
129+
const div = element
130+
const fetcher = store.fetcher as any
131+
132+
store.add(subject, predicate, target, subject.doc())
133+
fetcher.requested = {
134+
...fetcher.requested,
135+
[target.doc().uri]: 'requested'
136+
}
137+
138+
const nowOrWhenFetched = jest
139+
.spyOn(fetcher, 'nowOrWhenFetched')
140+
.mockImplementation(() => undefined as any)
141+
142+
attachmentList(dom, subject, div, {
143+
predicate,
144+
renderSupportingInfo: () => null
145+
})
146+
147+
expect(nowOrWhenFetched).toHaveBeenCalledWith(
148+
target.doc(),
149+
undefined,
150+
expect.any(Function)
151+
)
152+
})
153+
154+
it('adds one extra document-load refresh hook only when enabled', () => {
155+
const subject = sym('https://subject.example/profile/card#me')
156+
const predicate = ns.foaf('knows')
157+
const target = sym('https://friend.example/profile/card#me')
158+
const div = element
159+
const fetcher = store.fetcher as any
160+
161+
store.add(subject, predicate, target, subject.doc())
162+
fetcher.requested = {
163+
...fetcher.requested,
164+
[target.doc().uri]: 'requested'
165+
}
166+
167+
const nowOrWhenFetched = jest
168+
.spyOn(fetcher, 'nowOrWhenFetched')
169+
.mockImplementation(() => undefined as any)
170+
171+
attachmentList(dom, subject, div, {
172+
predicate,
173+
refreshOnDocumentLoad: false,
174+
renderSupportingInfo: () => null
175+
})
176+
const disabledCallCount = nowOrWhenFetched.mock.calls.length
177+
178+
nowOrWhenFetched.mockClear()
179+
div.innerHTML = ''
180+
181+
attachmentList(dom, subject, div, {
182+
predicate,
183+
renderSupportingInfo: () => null
184+
})
185+
186+
expect(nowOrWhenFetched.mock.calls.length).toBe(disabledCallCount + 1)
187+
})
119188
})
120189

121190
describe('button', () => {

0 commit comments

Comments
 (0)