Conversation
There was a problem hiding this comment.
Pull request overview
该 PR 主要围绕 Repo 的批量写入/更新路径做性能优化，通过减少 chrome.storage.local.set 的调用次数来降低存储写入开销，属于扩展后台数据访问层（DAO/Repo 基础设施）的性能改进。
Changes:
- 为
saveCacheAndStorage/saveStorage增加批量写入（Record）的重载能力。 - 扩展
Repo.updates：支持传入Record<string, Partial<T>>对不同 key 做不同 patch，并在缓存模式下合并为单次写入。 - 新增
src/app/repo/repo.test.ts，补齐 Repo 的核心 CRUD、缓存模式与批量更新用例测试。
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/app/repo/repo.ts | 增加批量写入重载；增强 updates 的批量更新能力以减少 storage 写入次数 |
| src/app/repo/repo.test.ts | 新增 Repo 全量单测，覆盖缓存/非缓存两套路径及新增 updates 形态 |
| function saveCacheAndStorage<T>(key: string, value: T): Promise<T>; | ||
| function saveCacheAndStorage<T>(items: Record<string, T>): Promise<void>; | ||
| function saveCacheAndStorage<T>(keyOrItems: string | Record<string, T>, value?: T): Promise<T | void> { | ||
| if (typeof keyOrItems === "string") { | ||
| return Promise.all([ | ||
| loadCache().then((cache) => { | ||
| cache[keyOrItems] = value; | ||
| }), |
There was a problem hiding this comment.
saveCacheAndStorage 的实现签名把 value 设成了可选 (value?: T)，但在 key 为 string 的分支里会直接写入 cache/storage。这样会削弱类型约束，后续如果有人误用 saveCacheAndStorage(key) 会把 undefined 写入缓存/存储且 TS 不一定能及时拦住。建议把实现签名拆成两个实现（或在 string 分支对 value 做断言/显式报错），保持 value 在该分支为必填。
src/app/repo/repo.ts
Outdated
| function saveStorage<T>(key: string, value: T): Promise<T>; | ||
| function saveStorage<T>(items: Record<string, T>): Promise<void>; | ||
| function saveStorage<T>(keyOrItems: string | Record<string, T>, value?: T): Promise<T | void> { | ||
| return new Promise((resolve) => { | ||
| chrome.storage.local.set( | ||
| { | ||
| [key]: value, | ||
| }, | ||
| () => { | ||
| const lastError = chrome.runtime.lastError; | ||
| if (lastError) { | ||
| console.error("chrome.runtime.lastError in chrome.storage.local.set:", lastError); | ||
| // 无视storage API错误，继续执行 | ||
| } | ||
| resolve(value); | ||
| const items = typeof keyOrItems === "string" ? { [keyOrItems]: value } : keyOrItems; | ||
| chrome.storage.local.set(items, () => { | ||
| const lastError = chrome.runtime.lastError; | ||
| if (lastError) { | ||
| console.error("chrome.runtime.lastError in chrome.storage.local.set:", lastError); | ||
| // 无视storage API错误，继续执行 | ||
| } | ||
| ); | ||
| resolve(value); | ||
| }); |
There was a problem hiding this comment.
saveStorage 同样把 value 设成了可选并且无论传入的是 key 还是 items 都 resolve(value)。这会让返回值语义在 items overload 下变得含糊（虽然最终是 undefined），也更容易掩盖误用导致的 undefined 写入。建议在实现里区分两种调用：string 分支强制要求 value，并在 items 分支 resolve(undefined)（或直接用 Promise）。
src/app/repo/repo.ts
Outdated
| return loadCache().then(async (cache) => { | ||
| if (Array.isArray(keysOrItems)) { | ||
| const saveRecord: Record<string, T> = {}; | ||
| const result: (T | false)[] = []; | ||
| keys.forEach((key) => { | ||
| const data = cache[key] as T; | ||
| if (data) { | ||
| Object.assign(data, val); | ||
| return saveCacheAndStorage(key, data) as Promise<T>; | ||
| saveRecord[key] = data; | ||
| result.push(data); | ||
| } else { | ||
| result.push(false); | ||
| } | ||
| return false; | ||
| }) | ||
| ) | ||
| ); | ||
| }); | ||
| return saveCacheAndStorage(saveRecord).then(() => result); | ||
| } | ||
| const saveRecord: Record<string, T> = {}; | ||
| const result: Record<string, T | false> = {}; | ||
| for (const key in keysOrItems) { | ||
| const cacheKey = this.joinKey(key); | ||
| const data = cache[cacheKey] as T; | ||
| if (data) { | ||
| Object.assign(data, keysOrItems[key]); | ||
| saveRecord[cacheKey] = data; | ||
| result[key] = data; | ||
| } else { | ||
| result[key] = false; | ||
| } | ||
| } | ||
| return saveCacheAndStorage(saveRecord).then(() => result); | ||
| }); |
There was a problem hiding this comment.
updates 在 useCache 分支里先 loadCache()，随后又调用 saveCacheAndStorage(saveRecord)；而 saveCacheAndStorage 内部会再次 loadCache() 并 Object.assign 一遍缓存，导致一次批量更新出现重复的 cache 加载/合并（与“优化性能”的目标相悖）。建议在已拿到 cache 且已就地修改 data 的情况下，直接对 chrome.storage.local.set(saveRecord) 做一次写入（并在 saveRecord 为空时直接返回 result，避免 set({}) 的额外开销）。另外这里的 then(async (cache)=>...) 没有 await，可去掉 async。
| beforeEach(async () => { | ||
| // 清空 storage | ||
| await chrome.storage.local.clear(); | ||
| repo = new TestRepo("test"); | ||
| }); |
There was a problem hiding this comment.
这里的 beforeEach 只清空了 chrome.storage.local，但 Repo 模块里有进程级的全局 cache（repo.ts 顶部的 cache/loadCachePromise）。在当前项目 vitest 以 --isolate=false 运行时，缓存模式用例之间会共享同一个内存 cache，导致测试相互污染/顺序依赖（例如 find/updates 可能读到上一个用例残留的 cached:* 数据）。建议在每个用例前额外重置 Repo 的模块级 cache：例如用 vi.resetModules()+重新 import（或在 repo.ts 暴露仅测试使用的 resetCache 方法并在这里调用）。
概述 Descriptions
变更内容 Changes
截图 Screenshots