Skip to content

增加: 支持 afterCheckUpdate 回调通知检查状态#531

Merged
sunnylqm merged 1 commit intoreactnativecn:masterfrom
Chenjiujiu:feat/afterCheckUpdate
Mar 27, 2026
Merged

增加: 支持 afterCheckUpdate 回调通知检查状态#531
sunnylqm merged 1 commit intoreactnativecn:masterfrom
Chenjiujiu:feat/afterCheckUpdate

Conversation

@Chenjiujiu
Copy link
Copy Markdown
Contributor

@Chenjiujiu Chenjiujiu commented Mar 27, 2026

变更说明

本次改动新增了 afterCheckUpdate 回调,用于在每次检查更新结束后统一通知业务侧当前结果。

目的

让业务侧在任何时候都可以知道最近一次检查是否已经执行完成,便于承接一些依赖检查结束时机的后续功能。

例如:

  • 检查完成后再展示后续 UI
  • 检查结束后再执行后续业务逻辑
  • 区分本次检查是成功、跳过还是失败

回调参数

afterCheckUpdate 会收到一个 state 对象,包含以下字段:

  • status
  • result
  • error

其中 status 支持以下值:

  • completed
  • skipped
  • error

设计说明

这次改动采用“检查结束通知”的方式,而不是在 useUpdate() 中额外维护一份状态,尽量减少对现有更新流程的影响。

同时保留了现有 checkUpdateupdateInfo 等原有逻辑,不改变已有使用方式。

验证

已通过以下检查:

  • bun test src/__tests__
  • bun lint

Summary by CodeRabbit

Release Notes

  • New Features
    • Added afterCheckUpdate optional 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.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 27, 2026

📝 Walkthrough

Walkthrough

A new afterCheckUpdate callback hook has been added to the update check flow. The callback fires after each checkUpdate() operation with a state object indicating whether the check was skipped, completed successfully, or encountered an error, enabling post-check processing without altering existing logic.

Changes

Cohort / File(s) Summary
Type Definitions
src/type.ts
Added UpdateCheckState interface with status, optional result, and optional error properties. Extended ClientOptions with optional afterCheckUpdate hook property.
Client Implementation
src/client.ts
Added notifyAfterCheckUpdate() method to conditionally invoke the callback and log failures. Updated checkUpdate() to call this method with appropriate state across multiple code paths: early returns (assertions, beforeCheckUpdate check), successful completion (cached or fetched), and error handling.
Provider Integration
src/provider.tsx
Updated checkUpdate() to notify after skipping duplicate checks occurring within 1 second.
Test Suite
src/__tests__/client.test.ts
Added three test cases verifying callback invocation with correct state objects for skipped, completed, and error scenarios.

Sequence Diagram

sequenceDiagram
    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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Whiskers twitch with joy, the update dance is done,
After checks conclude, our callbacks run and run!
Skipped or completed, errors caught with care,
State flows through the warren—notifications everywhere!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: adding support for an afterCheckUpdate callback to notify check status, which aligns with the implementation across all modified files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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 (lastRespJson reuse 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

📥 Commits

Reviewing files that changed from the base of the PR and between c2c66b0 and 823084f.

📒 Files selected for processing (4)
  • src/__tests__/client.test.ts
  • src/client.ts
  • src/provider.tsx
  • src/type.ts

@sunnylqm sunnylqm merged commit 5b81b4d into reactnativecn:master Mar 27, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants