增加: 支持 afterCheckUpdate 回调通知检查状态#531
Conversation
📝 WalkthroughWalkthroughA new Changes
Sequence DiagramsequenceDiagram
actor User
participant CP as checkUpdate()
participant CB as beforeCheckUpdate
participant FE as fetch/cache
participant NF as notifyAfterCheckUpdate
participant AH as afterCheckUpdate callback
User->>CP: invoke checkUpdate()
alt Assertion fails
CP->>NF: status: 'skipped'
NF->>AH: invoke callback
else beforeCheckUpdate returns false
CP->>CB: call hook
CB-->>CP: false
CP->>NF: status: 'skipped'
NF->>AH: invoke callback
else Fetch/cache succeeds
CP->>FE: fetch or use cache
FE-->>CP: result
CP->>NF: status: 'completed', result
NF->>AH: invoke callback
NF-->>CP: return result
else Error occurs
CP->>FE: fetch fails
FE-->>CP: error
CP->>NF: status: 'error', error
NF->>AH: invoke callback
NF-->>CP: rethrow or fallback
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/__tests__/client.test.ts (1)
139-201: Nice coverage for the three primary callback states.Consider adding one more case for the deduped cached branch (
lastRespJsonreuse within 5s), especially when the shared in-flight request rejects, to lock down callback behavior for concurrent checks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/client.test.ts` around lines 139 - 201, Add a test that verifies checkUpdate returns the cached lastRespJson (within 5s) and calls afterCheckUpdate with {status: 'completed', result} even if a shared in-flight request rejects: create a Pushy instance (from Pushy), set its internal lastRespJson and lastRespTs to a recent timestamp (within 5s), set an in-flight promise that rejects (client['_inFlightPromise'] = Promise.reject(new Error('boom')) or make fetch throw), provide an afterCheckUpdate mock, call client.checkUpdate(), expect it to resolve to the cached result and expect afterCheckUpdate to have been called with status 'completed' and the cached result; use symbols lastRespJson, lastRespTs, _inFlightPromise, checkUpdate, and afterCheckUpdate to locate and implement the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/client.ts`:
- Around line 364-366: The await of the shared promise this.lastRespJson can
reject outside the method’s try/catch and skip the notifyAfterCheckUpdate/error
path; wrap the await in the method’s existing try/catch (or add a local
try/catch just around awaiting this.lastRespJson) so any rejection calls
notifyAfterCheckUpdate({ status: 'error', error })/afterCheckUpdate equivalent
and respects the throwError flag (i.e., only rethrow when throwError is true);
refer to this.lastRespJson and notifyAfterCheckUpdate in your fix so the error
branch always runs when the cached in-flight promise rejects.
---
Nitpick comments:
In `@src/__tests__/client.test.ts`:
- Around line 139-201: Add a test that verifies checkUpdate returns the cached
lastRespJson (within 5s) and calls afterCheckUpdate with {status: 'completed',
result} even if a shared in-flight request rejects: create a Pushy instance
(from Pushy), set its internal lastRespJson and lastRespTs to a recent timestamp
(within 5s), set an in-flight promise that rejects (client['_inFlightPromise'] =
Promise.reject(new Error('boom')) or make fetch throw), provide an
afterCheckUpdate mock, call client.checkUpdate(), expect it to resolve to the
cached result and expect afterCheckUpdate to have been called with status
'completed' and the cached result; use symbols lastRespJson, lastRespTs,
_inFlightPromise, checkUpdate, and afterCheckUpdate to locate and implement the
test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 84bbd527-8be7-4a73-bfa3-14bdffedee58
📒 Files selected for processing (4)
src/__tests__/client.test.tssrc/client.tssrc/provider.tsxsrc/type.ts
变更说明
本次改动新增了
afterCheckUpdate回调,用于在每次检查更新结束后统一通知业务侧当前结果。目的
让业务侧在任何时候都可以知道最近一次检查是否已经执行完成,便于承接一些依赖检查结束时机的后续功能。
例如:
回调参数
afterCheckUpdate会收到一个state对象,包含以下字段:statusresulterror其中
status支持以下值:completedskippederror设计说明
这次改动采用“检查结束通知”的方式,而不是在
useUpdate()中额外维护一份状态,尽量减少对现有更新流程的影响。同时保留了现有
checkUpdate、updateInfo等原有逻辑,不改变已有使用方式。验证
已通过以下检查:
bun test src/__tests__bun lintSummary by CodeRabbit
Release Notes
afterCheckUpdateoptional callback hook to client configuration. Automatically invoked after each update check attempt, providing status updates indicating whether the check completed successfully, was skipped, or encountered an error, along with associated result or error data.