chore: housekeeping: clean up code and remove unused files#533
Open
NekoCharm01 wants to merge 4 commits intomasterfrom
Open
chore: housekeeping: clean up code and remove unused files#533NekoCharm01 wants to merge 4 commits intomasterfrom
NekoCharm01 wants to merge 4 commits intomasterfrom
Conversation
Contributor
Reviewer's Guide重构应用的状态管理和构建配置,使其与 Nuxt 4 和基于 composable 的 store 方案保持一致;清理未使用的类型/工具代码;统一 Pug/ARIA 格式;并升级若干前端依赖。 基于 composable 的 Pinia stores 与 Pixiv client 更新后的类图classDiagram
class PixivWebClient {
+number timeout
+getFollowLatest(p)
+getRankings(params)
+searchIllust(params)
+getUserDetail(userId)
+getUserIllusts(userId, params)
+getIllustDetail(illustId)
+getRecommend(params)
}
class usePixivClientStore {
+PixivWebClient client
}
class useUserStore {
+boolean isLoggedIn
+string userId
+string userPixivId
+string userName
+string userProfileImg
+string userProfileImgBig
+loginWithToken(token)
+logout()
}
class useSideNavStore {
+boolean isOpened
+open()
+close()
+toggle()
}
class useArtworkStore {
+Map~string_Artwork~ artworkCache
+Map~string_ArtworkGallery~ galleryCache
+Artwork current
+boolean loading
+fetchArtwork(id)
+fetchGallery(id)
+fetchRecommendations(id)
}
class useFollowingStore {
+UserListItem[] publicList
+UserListItem[] hiddenList
+ArtworkInfo[] latestIllusts
+boolean isLoadingPublic
+boolean isLoadingHidden
+boolean latestLoading
+number latestNextPage
+fetchPublicList()
+fetchHiddenList()
+fetchLatest()
}
class useHomeStore {
+ArtworkInfo randomBg
+boolean loading
+fetchRandomBackground()
}
class useRankingStore {
+Date date
+string mode
+ArtworkRank[] list
+boolean loading
+fetchRanking(mode, date)
}
class useSearchStore {
+ArtworkInfo[] results
+number total
+boolean loading
+string keyword
+number page
+search(keyword, page)
}
class useUserArtworksStore {
+string[] allIds
+ArtworkInfo[] currentPageItems
+number page
+boolean loading
+fetchUserArtworks(userId, category, page)
}
class useUserProfileStore {
+Map~string_User~ userCache
+ArtworkInfo[] illusts
+ArtworkInfo[] mangas
+boolean loading
+fetchUser(userId)
+fetchUserIllusts(userId)
+fetchUserMangas(userId)
}
usePixivClientStore --> PixivWebClient : wraps
useUserStore --> usePixivClientStore : may_use
useArtworkStore --> usePixivClientStore : uses_client
useFollowingStore --> usePixivClientStore : uses_client
useHomeStore --> usePixivClientStore : uses_client
useRankingStore --> usePixivClientStore : uses_client
useSearchStore --> usePixivClientStore : uses_client
useUserArtworksStore --> usePixivClientStore : uses_client
useUserProfileStore --> usePixivClientStore : uses_client
class useUserStore{
}
class useSideNavStore{
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your Experience打开你的 dashboard 以:
Getting HelpOriginal review guide in EnglishReviewer's GuideRefactors the app’s state management and build configuration to align with Nuxt 4 + composables-based stores, cleans up unused/types/utility code, standardizes Pug/ARIA formatting, and bumps several frontend dependencies. Updated class diagram for composables-based Pinia stores and Pixiv clientclassDiagram
class PixivWebClient {
+number timeout
+getFollowLatest(p)
+getRankings(params)
+searchIllust(params)
+getUserDetail(userId)
+getUserIllusts(userId, params)
+getIllustDetail(illustId)
+getRecommend(params)
}
class usePixivClientStore {
+PixivWebClient client
}
class useUserStore {
+boolean isLoggedIn
+string userId
+string userPixivId
+string userName
+string userProfileImg
+string userProfileImgBig
+loginWithToken(token)
+logout()
}
class useSideNavStore {
+boolean isOpened
+open()
+close()
+toggle()
}
class useArtworkStore {
+Map~string_Artwork~ artworkCache
+Map~string_ArtworkGallery~ galleryCache
+Artwork current
+boolean loading
+fetchArtwork(id)
+fetchGallery(id)
+fetchRecommendations(id)
}
class useFollowingStore {
+UserListItem[] publicList
+UserListItem[] hiddenList
+ArtworkInfo[] latestIllusts
+boolean isLoadingPublic
+boolean isLoadingHidden
+boolean latestLoading
+number latestNextPage
+fetchPublicList()
+fetchHiddenList()
+fetchLatest()
}
class useHomeStore {
+ArtworkInfo randomBg
+boolean loading
+fetchRandomBackground()
}
class useRankingStore {
+Date date
+string mode
+ArtworkRank[] list
+boolean loading
+fetchRanking(mode, date)
}
class useSearchStore {
+ArtworkInfo[] results
+number total
+boolean loading
+string keyword
+number page
+search(keyword, page)
}
class useUserArtworksStore {
+string[] allIds
+ArtworkInfo[] currentPageItems
+number page
+boolean loading
+fetchUserArtworks(userId, category, page)
}
class useUserProfileStore {
+Map~string_User~ userCache
+ArtworkInfo[] illusts
+ArtworkInfo[] mangas
+boolean loading
+fetchUser(userId)
+fetchUserIllusts(userId)
+fetchUserMangas(userId)
}
usePixivClientStore --> PixivWebClient : wraps
useUserStore --> usePixivClientStore : may_use
useArtworkStore --> usePixivClientStore : uses_client
useFollowingStore --> usePixivClientStore : uses_client
useHomeStore --> usePixivClientStore : uses_client
useRankingStore --> usePixivClientStore : uses_client
useSearchStore --> usePixivClientStore : uses_client
useUserArtworksStore --> usePixivClientStore : uses_client
useUserProfileStore --> usePixivClientStore : uses_client
class useUserStore{
}
class useSideNavStore{
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - 我发现了两个问题,并给出了一些整体性的反馈:
- 在
DeferLoad.vue中,useIntersectionObserver回调现在直接访问entries[0];建议在!entries.length时尽早 return,这样可以避免当观察器在 entries 为空数组时触发而导致的潜在 undefined 访问。 - 在
ErrorPage.vue中,当随机到相同的消息时,randomMsg()会递归调用自身;如果增加一个msgList.length <= 1的短路判断,就可以避免不必要的递归(以及在只有一条消息时可能出现的无限递归)。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- In `DeferLoad.vue`, the `useIntersectionObserver` callback now indexes `entries[0]` directly; consider early-returning when `!entries.length` to avoid potential undefined access if the observer fires with an empty entries array.
- In `ErrorPage.vue`, `randomMsg()` recursively calls itself when the same message is chosen; adding a short-circuit for `msgList.length <= 1` would prevent unnecessary recursion (and a potential infinite loop if there is ever only a single message).
## Individual Comments
### Comment 1
<location path="nuxt.config.ts" line_range="44-47" />
<code_context>
i18n: {
locales: [{ code: 'zh-Hans', file: 'zh-Hans.json' }],
defaultLocale: 'zh-Hans',
- lazy: true,
- langDir: '../app/locales',
strategy: 'no_prefix',
},
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `file` in `locales` without `langDir` may break locale loading after moving config from `app/locales`.
The config still references `file: 'zh-Hans.json'`, but `langDir` has been removed. In @nuxtjs/i18n v10, using `file` relies on `langDir` (defaulting to `locales/` at the project root), whereas this was previously set to `../app/locales`. Unless the JSON file has been moved to the default directory, this will prevent the locale from loading. Please either restore `langDir` to the folder containing `zh-Hans.json` or switch to inlined messages if the file is no longer used.
</issue_to_address>
### Comment 2
<location path="app/components/ErrorPage.vue" line_range="77" />
<code_context>
const msg = ref('')
function randomMsg(): void {
- const newValue = msgList[Math.floor(Math.random() * msgList.length)]
+ const newValue = msgList[Math.floor(Math.random() * msgList.length)]!
if (newValue !== msg.value) {
msg.value = newValue
</code_context>
<issue_to_address>
**suggestion:** Non-null assertion on `msgList[...]` hides potential empty-list issues and keeps the existing recursion pitfall.
Using `!` just silences TS but still risks a runtime error if `msgList` is empty and unnecessary recursion if it has only one element. Consider returning early when `msgList.length === 0`, and in the single-item case, just assign that item directly. This removes the need for `!` and makes the function more robust to future changes in `msgList`.
Suggested implementation:
```
const msg = ref('')
function randomMsg(): void {
const { length } = msgList
if (length === 0) {
// Nothing to show
return
}
if (length === 1) {
// Only one possible message, avoid recursion/extra work
msg.value = msgList[0]
return
}
const newIndex = Math.floor(Math.random() * length)
const newValue = msgList[newIndex]
if (newValue !== msg.value) {
msg.value = newValue
} else {
```
The current `else { ... }` branch likely contains a recursive `randomMsg()` call to avoid repeating the same message. With the above `length === 1` guard, that recursion should no longer be needed for the single-item case. Consider updating the `else` branch to either:
1. Return early (do nothing if the same message is randomly chosen), or
2. Select a different index deterministically (e.g. `(newIndex + 1) % length`) instead of recursively calling `randomMsg()`.
Adjust the `else` block accordingly to avoid unbounded recursion if `msgList` is ever changed back to a single-element list.
</issue_to_address>帮我变得更有用!请对每条评论点 👍 或 👎,我会根据你的反馈改进后续的 Review。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- In
DeferLoad.vue, theuseIntersectionObservercallback now indexesentries[0]directly; consider early-returning when!entries.lengthto avoid potential undefined access if the observer fires with an empty entries array. - In
ErrorPage.vue,randomMsg()recursively calls itself when the same message is chosen; adding a short-circuit formsgList.length <= 1would prevent unnecessary recursion (and a potential infinite loop if there is ever only a single message).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `DeferLoad.vue`, the `useIntersectionObserver` callback now indexes `entries[0]` directly; consider early-returning when `!entries.length` to avoid potential undefined access if the observer fires with an empty entries array.
- In `ErrorPage.vue`, `randomMsg()` recursively calls itself when the same message is chosen; adding a short-circuit for `msgList.length <= 1` would prevent unnecessary recursion (and a potential infinite loop if there is ever only a single message).
## Individual Comments
### Comment 1
<location path="nuxt.config.ts" line_range="44-47" />
<code_context>
i18n: {
locales: [{ code: 'zh-Hans', file: 'zh-Hans.json' }],
defaultLocale: 'zh-Hans',
- lazy: true,
- langDir: '../app/locales',
strategy: 'no_prefix',
},
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `file` in `locales` without `langDir` may break locale loading after moving config from `app/locales`.
The config still references `file: 'zh-Hans.json'`, but `langDir` has been removed. In @nuxtjs/i18n v10, using `file` relies on `langDir` (defaulting to `locales/` at the project root), whereas this was previously set to `../app/locales`. Unless the JSON file has been moved to the default directory, this will prevent the locale from loading. Please either restore `langDir` to the folder containing `zh-Hans.json` or switch to inlined messages if the file is no longer used.
</issue_to_address>
### Comment 2
<location path="app/components/ErrorPage.vue" line_range="77" />
<code_context>
const msg = ref('')
function randomMsg(): void {
- const newValue = msgList[Math.floor(Math.random() * msgList.length)]
+ const newValue = msgList[Math.floor(Math.random() * msgList.length)]!
if (newValue !== msg.value) {
msg.value = newValue
</code_context>
<issue_to_address>
**suggestion:** Non-null assertion on `msgList[...]` hides potential empty-list issues and keeps the existing recursion pitfall.
Using `!` just silences TS but still risks a runtime error if `msgList` is empty and unnecessary recursion if it has only one element. Consider returning early when `msgList.length === 0`, and in the single-item case, just assign that item directly. This removes the need for `!` and makes the function more robust to future changes in `msgList`.
Suggested implementation:
```
const msg = ref('')
function randomMsg(): void {
const { length } = msgList
if (length === 0) {
// Nothing to show
return
}
if (length === 1) {
// Only one possible message, avoid recursion/extra work
msg.value = msgList[0]
return
}
const newIndex = Math.floor(Math.random() * length)
const newValue = msgList[newIndex]
if (newValue !== msg.value) {
msg.value = newValue
} else {
```
The current `else { ... }` branch likely contains a recursive `randomMsg()` call to avoid repeating the same message. With the above `length === 1` guard, that recursion should no longer be needed for the single-item case. Consider updating the `else` branch to either:
1. Return early (do nothing if the same message is randomly chosen), or
2. Select a different index deterministically (e.g. `(newIndex + 1) % length`) instead of recursively calling `randomMsg()`.
Adjust the `else` block accordingly to avoid unbounded recursion if `msgList` is ever changed back to a single-element list.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
重构应用结构和配置,将 Pinia store 迁移到 composable,并移除未使用的类型和工具定义。
Bug 修复:
增强:
@bg-dev/nuxt-naiveui,将图标相关处理移动到 modules 配置中,并通过 TypeScript 设置来配置 Vue Pug 支持。构建:
axios、vue、@tabler/icons-vue、@types/node和sass-embedded升级到较新的补丁版本,并新增@bg-dev/nuxt-naiveui作为开发依赖。nuxtjs-naive-ui以及基于 unplugin 的 Naive UI 自动导入配置,改为使用新的 Nuxt Naive UI 模块。tsconfig,并将 Vue 编译器插件相关配置移动到nuxt.config.ts中。Original summary in English
Summary by Sourcery
Refactor application structure and configuration, migrate Pinia stores to composables, and remove unused types and utility definitions.
Bug Fixes:
Enhancements:
Build: