Summary
frontend/src/_ui/Avatar/index.jsx never revokes the blob URLs it creates, because the cleanup function is written but never returned. Separately, the same effect has no guard against a stale response, so a row in the virtualized user list can end up showing a different user's picture.
The code
React.useEffect(() => {
async function fetchAvatar() {
const blob = await userService.getAvatar(avatarId);
setAvatar(URL.createObjectURL(blob));
}
if (avatarId) fetchAvatar();
() => avatar && URL.revokeObjectURL(avatar);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [avatarId]);
Line 18 is missing a return. As written it constructs an arrow function and discards it, so URL.revokeObjectURL is never reached. The intent is clearly there, it just is not wired up. Every other place in the frontend that creates a blob URL does revoke it properly, Camera.jsx, PDF.jsx, Chat/index.js, _lib/generate-file.js and BulkUploadDrawer, which is what made this one stand out.
no-unused-expressions is not enabled in eslint.config.mjs, so nothing flagged it.
Impact
Avatar is rendered per row in UsersTable.jsx and in VirtualizedUserList.jsx. Because the list is virtualized, avatarId changes on the same mounted component as rows are recycled during scrolling, so this is not a one-off allocation. Each change fetches a new blob and leaks the previous URL for the lifetime of the page.
The same recycling causes the second problem. There is no ignore flag, so if a request for the previous avatarId resolves after the component has moved to a new one, the late response calls setAvatar and overwrites the correct picture. In a members list that means a row showing the wrong person's avatar, which is more confusing than a blank one.
Expected vs actual
Expected: the object URL for an avatar is released when the avatar changes or the component unmounts, and a response that arrives after avatarId has moved on is discarded.
Actual: no URL is ever released, and a late response wins.
Notes
I have a fix ready with tests, three cases covering revoke-on-unmount, revoke-on-id-change and ignoring the stale response. All three fail against main and pass with the change. I am happy to open the PR, or to leave it if you would rather assign this elsewhere. Just say which.
One thing I ran into while writing the tests, which is separate from this issue and I am not proposing to fix here: the frontend jest config sets "testEnvironment": "jest-environment-jsdom" but jest-environment-jsdom is not in frontend/package.json. Since Jest 28 it is not bundled, so on a fresh clone npx jest fails with Test environment jest-environment-jsdom cannot be found before running anything. Existing tests such as src/_helpers/__tests__/validateName.test.js hit the same wall. No CI workflow runs frontend jest so nothing is red because of it, but it does mean the suite cannot be run locally without installing that package by hand. Happy to file it separately if useful.
Summary
frontend/src/_ui/Avatar/index.jsxnever revokes the blob URLs it creates, because the cleanup function is written but never returned. Separately, the same effect has no guard against a stale response, so a row in the virtualized user list can end up showing a different user's picture.The code
Line 18 is missing a
return. As written it constructs an arrow function and discards it, soURL.revokeObjectURLis never reached. The intent is clearly there, it just is not wired up. Every other place in the frontend that creates a blob URL does revoke it properly,Camera.jsx,PDF.jsx,Chat/index.js,_lib/generate-file.jsandBulkUploadDrawer, which is what made this one stand out.no-unused-expressionsis not enabled ineslint.config.mjs, so nothing flagged it.Impact
Avataris rendered per row inUsersTable.jsxand inVirtualizedUserList.jsx. Because the list is virtualized,avatarIdchanges on the same mounted component as rows are recycled during scrolling, so this is not a one-off allocation. Each change fetches a new blob and leaks the previous URL for the lifetime of the page.The same recycling causes the second problem. There is no ignore flag, so if a request for the previous
avatarIdresolves after the component has moved to a new one, the late response callssetAvatarand overwrites the correct picture. In a members list that means a row showing the wrong person's avatar, which is more confusing than a blank one.Expected vs actual
Expected: the object URL for an avatar is released when the avatar changes or the component unmounts, and a response that arrives after
avatarIdhas moved on is discarded.Actual: no URL is ever released, and a late response wins.
Notes
I have a fix ready with tests, three cases covering revoke-on-unmount, revoke-on-id-change and ignoring the stale response. All three fail against
mainand pass with the change. I am happy to open the PR, or to leave it if you would rather assign this elsewhere. Just say which.One thing I ran into while writing the tests, which is separate from this issue and I am not proposing to fix here: the frontend jest config sets
"testEnvironment": "jest-environment-jsdom"butjest-environment-jsdomis not infrontend/package.json. Since Jest 28 it is not bundled, so on a fresh clonenpx jestfails withTest environment jest-environment-jsdom cannot be foundbefore running anything. Existing tests such assrc/_helpers/__tests__/validateName.test.jshit the same wall. No CI workflow runs frontend jest so nothing is red because of it, but it does mean the suite cannot be run locally without installing that package by hand. Happy to file it separately if useful.