Skip to content

ユーザーアイコンのキャッシュ管理の責務を移動 #195

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.

Sign up for GitHub

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

Open
kitsne241 wants to merge 10 commits into
base: main
Choose a base branch
from
+42 −73
Open

ユーザーアイコンのキャッシュ管理の責務を移動 #195

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7b99184
UserIcon コンポーネントで Query を使用しない
kitsne241 Jan 25, 2026
81896aa
アイコン画像のキャッシュ設定を追加
kitsne241 Jan 25, 2026
2109bf9
image-proxy.trap.jp から画像を利用
kitsne241 Jan 26, 2026
e1fa148
キャッシュ戦略を SWR に変更
kitsne241 Jan 26, 2026
85547f4
Revert "キャッシュ戦略を SWR に変更"
kitsne241 Jan 26, 2026
a4a4cd4
Revert "image-proxy.trap.jp から画像を利用"
kitsne241 Jan 26, 2026
34ee59a
Merge remote-tracking branch 'origin/main' into feat/user-icon-sw-cache
kitsne241 Feb 14, 2026
6382e5a
レビュー箇所を変更
kitsne241 Feb 14, 2026
417dabb
アイコンが正常にロードされないので loading="lazy" を削除
kitsne241 Feb 14, 2026
ff4b98f
v-img に乗り換え
kitsne241 Feb 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/api/queries/keys.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,7 @@ const campKeys = {
dashboard: (id: number) => ['camps', 'detail', id, 'dashboard'] as const,
}

const iconKeys = {
all: ['icons'] as const,
user: (id: string) => ['icons', 'user', id] as const,
}

export const qk = {
me: meKeys,
camps: campKeys,
icons: iconKeys,
}
98 changes: 35 additions & 63 deletions src/components/generic/UserIcon.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,86 +1,58 @@
<script setup lang="ts">
import { computed, ref, useAttrs } from 'vue'
import { useQuery } from '@tanstack/vue-query'
import { qk } from '@/api/queries/keys'
import { computed, ref } from 'vue'
import { useUserStore } from '@/store'

const props = defineProps<{ id?: string; size: number; idTooltip?: boolean }>()
// idTooltip ... クリック時に Tooltip で ID を表示するかどうか
const userId = props.id ?? useUserStore().user.id // id が指定されていない場合は自分のアイコンを表示
const props = defineProps<{
id?: string
size: number
/** クリック時に Tooltip で ID を表示 */
idTooltip?: boolean
}>()

const attrs = useAttrs()
const iconRef = ref<HTMLElement | undefined>()

const imageStyle = {
width: `${props.size}px`,
height: `${props.size}px`,
objectFit: 'contain' as const,
borderRadius: '50%',
display: 'block',
cursor: props.idTooltip ? 'pointer' : 'default',
}

const directUrl = `https://q.trap.jp/api/v3/public/icon/${userId}`
const userId = computed(() => props.id ?? useUserStore().user.id)
const iconUrl = computed(() => `https://q.trap.jp/api/v3/public/icon/${userId.value}`)
const cursorStyle = computed(() => ({ cursor: props.idTooltip ? 'pointer' : 'default' }))

const {
data: cachedIconUrl,
isLoading,
isFetching,
isError,
} = useQuery<string, Error>({
queryKey: qk.icons.user(userId),
staleTime: 24 * 60 * 60_000, // 24h
gcTime: 24 * 60 * 60_000, // 24h
retry: 0,
// データURLで保存する
queryFn: async () => {
const res = await fetch(directUrl)
if (!res.ok) throw new Error(`アイコンを取得できませんでした: ${res.status}`)
// 画像のロード状態を管理
const isLoading = ref(true)
const hasError = ref(false)
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loading and error states are not reset when the userId prop changes. When the id prop changes (e.g., when rendering a different user's icon), the component will continue to show the skeleton loader or error state from the previous user until the new image loads or fails. This can cause visual inconsistencies where an old skeleton or error state is displayed while the new image is loading.

Add a watch effect to reset these states when userId changes.

Copilot uses AI. Check for mistakes.
const showSkeleton = computed(() => isLoading.value || hasError.value)

const blob = await res.blob()
const dataUrl = await new Promise<string>((resolve, reject) => {
const reader = new FileReader()
reader.onloadend = () => resolve(reader.result as string)
reader.onerror = () => reject(new Error('アイコンの変換に失敗しました'))
reader.readAsDataURL(blob)
})
return dataUrl
},
})

const showSkeleton = computed(
() => !cachedIconUrl.value || isLoading.value || isFetching.value || isError.value,
)
const handleLoad = () => {
isLoading.value = false
}

const tooltipText = `@${userId}`
const handleError = () => {
isLoading.value = false
hasError.value = true
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

エラー後/ID変更後にスケルトンが固定される可能性があります

hasError を解除する経路がなく、iconUrl が変わっても isLoading/hasError がリセットされないため、一時的な失敗やID切替後にアイコンが表示されなくなります。iconUrl 変更時に状態をリセットし、成功時に hasError をクリアしてください。

🛠️ 修正案 
-import { computed, ref } from 'vue'
+import { computed, ref, watch } from 'vue'
@@
-const iconUrl = computed(() => `https://q.trap.jp/api/v3/public/icon/${userId.value}`)
+const iconUrl = computed(() =>
+  userId.value ? `https://q.trap.jp/api/v3/public/icon/${userId.value}` : ''
+)
@@
 const showSkeleton = computed(() => isLoading.value || hasError.value)
+
+watch(iconUrl, () => {
+  isLoading.value = true
+  hasError.value = false
+})
@@
 const handleLoad = () => {
   isLoading.value = false
+  hasError.value = false
 }
🤖 Prompt for AI Agents 
In `@src/components/generic/UserIcon.vue` around lines 2 - 29, The component
currently never clears hasError and never resets loading state when iconUrl
changes; update handleLoad to also set hasError.value = false so a successful
load clears the error, and add a watcher on iconUrl (the computed used to build
the image URL) to reset isLoading.value = true and hasError.value = false
whenever iconUrl changes (so switching IDs or retries will re-run loading).
Reference: hasError, isLoading, iconUrl, handleLoad, handleError.

}
</script>

<template>
<template v-if="showSkeleton">
<v-avatar :size="size">
<v-skeleton-loader type="image" class="w-100 h-100" />
<div class="rounded-circle overflow-hidden">
<v-avatar ref="iconRef" :size="size" class="d-block" :style="cursorStyle">
<v-skeleton-loader v-if="showSkeleton" type="image" class="w-100 h-100" />
<img
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of v-bind="attrs" means that any attributes passed to the UserIcon component (like class, style, or data attributes) will no longer be forwarded to the appropriate element. While the current usages in the codebase mainly pass standard props, this change could break existing or future usages that rely on attribute inheritance.

If attribute forwarding is not needed, consider adding inheritAttrs: false to the script setup to make this explicit. Otherwise, consider applying v-bind="$attrs" to an appropriate element.

Copilot uses AI. Check for mistakes.
v-show="!showSkeleton"
class="w-100 h-100"
tabindex="0"
:src="iconUrl"
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The img element no longer has the loading="lazy" attribute that was present in the original implementation. Without this attribute, all user icons will be loaded immediately when the page renders, which could impact page load performance when many user icons are displayed (e.g., in the room info view or answer lists). The Service Worker cache will help with repeated loads, but the initial page load could be slower.

Consider adding loading="lazy" back to the img element to preserve lazy loading behavior.

Suggested change
:src="iconUrl"
:src="iconUrl"
loading="lazy"

Copilot uses AI. Check for mistakes.
@load="handleLoad"
@error="handleError"
/>
</v-avatar>
</template>

<template v-else>
<img
ref="iconRef"
tabindex="0"
v-bind="attrs"
:style="imageStyle"
:src="cachedIconUrl"
loading="lazy"
/>

<v-tooltip
:activator="iconRef"
:open-on-hover="idTooltip"
:open-on-click="idTooltip"
:open-delay="1000"
location="top"
>
<span class="text-white font-weight-medium">{{ tooltipText }}</span>
<span class="text-white font-weight-medium">{{ `@${userId}` }}</span>
</v-tooltip>
Comment on lines 32 to 41
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The v-tooltip is positioned outside the v-avatar but references it via activator. This creates a DOM structure where the tooltip's visual container (div.rounded-circle) wraps both the avatar and the tooltip element. However, with the current structure, the tooltip will always be rendered in the DOM even when idTooltip is false, which could have performance implications in lists with many UserIcon components.

Consider moving the v-tooltip inside the v-avatar or conditionally rendering it only when idTooltip is true.

Copilot uses AI. Check for mistakes.
</template>
</div>
</template>

<style module>
Expand Down
16 changes: 16 additions & 0 deletions vite.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,22 @@ export default defineConfig(({ mode }) => {
workbox: {
globPatterns: ['**/*.{js,css,html,svg,png,jpg,webp,woff2}'],
navigateFallbackDenylist: [/^\/api/, /^\/login/],
runtimeCaching: [
{
urlPattern: /^https:\/\/q\.trap\.jp\/api\/v3\/public\/icon\/.*/,
handler: 'CacheFirst',
options: {
cacheName: 'user-icons',
expiration: {
maxEntries: 200,
maxAgeSeconds: 7 * 24 * 60 * 60, // 1 週間
},
cacheableResponse: {
statuses: [0, 200],
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cacheableResponse includes status 0, which is typically for opaque responses from cross-origin requests. However, the URL pattern matches https://q.trap.jp/api/v3/public/icon/*, which appears to be a same-origin or CORS-enabled API endpoint. If the API endpoint properly returns CORS headers and status 200 responses, including status 0 may be unnecessary and could cache failed opaque responses.

Verify whether status 0 is actually needed for this endpoint, or if only status 200 should be cached.

Suggested change
statuses: [0, 200],
statuses: [200],

Copilot uses AI. Check for mistakes.
},
},
},
],
},
devOptions: { enabled: true },
}),
Expand Down
Loading