-
Notifications
You must be signed in to change notification settings - Fork 1
feat(uikit): add useAsyncPolling composable #1262
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.
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
base: main
Are you sure you want to change the base?
Conversation
dbolotin
commented
Nov 12, 2025
- Implement useAsyncPolling composable for async polling with controlled timing
- Uses setTimeout (not setInterval) to wait for async callback completion
- Supports minInterval (time between starts) and minDelay (delay after completion)
- Integrates AbortController for proper cancellation
- Provides pause/resume controls with isActive state
- Includes error handling with lastError ref and pauseOnError option
- Add comprehensive test suite with 27 tests covering all features
- Export from uikit package
- Implement useAsyncPolling composable for async polling with controlled timing - Uses setTimeout (not setInterval) to wait for async callback completion - Supports minInterval (time between starts) and minDelay (delay after completion) - Integrates AbortController for proper cancellation - Provides pause/resume controls with isActive state - Includes error handling with lastError ref and pauseOnError option - Add comprehensive test suite with 27 tests covering all features - Export from uikit package
|
Summary of ChangesHello @dbolotin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new useAsyncPolling composable, which provides a robust mechanism for asynchronous polling with fine-grained control over timing, cancellation, and error handling. The implementation is well-structured, thoroughly documented, and accompanied by a comprehensive test suite. My review focuses on improving the clarity of the public API documentation and enhancing the maintainability of the tests. I've suggested a minor update to the JSDoc for the resume function to make its behavior more explicit, and a refactoring of a test case to improve its readability.
| it('should not schedule next iteration if paused during callback', async () => { | ||
| let shouldPause = false; | ||
| const callback = vi.fn(async () => { | ||
| await new Promise((resolve) => setTimeout(resolve, 30)); | ||
| }); | ||
|
|
||
| const { pause } = useAsyncPolling(callback, { minInterval: 100 }); | ||
|
|
||
| await vi.advanceTimersByTimeAsync(100); | ||
| expect(callback).toHaveBeenCalledTimes(1); | ||
|
|
||
| shouldPause = true; | ||
| await vi.advanceTimersByTimeAsync(100); | ||
|
|
||
| if (shouldPause) { | ||
| pause(); | ||
| } | ||
|
|
||
| await vi.advanceTimersByTimeAsync(200); | ||
| expect(callback).toHaveBeenCalledTimes(2); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test case is functionally correct, but its implementation is a bit confusing and could be simplified for better readability and maintainability. The shouldPause variable is redundant as it's always true when the if condition is checked. The flow of advancing timers and then pausing can be made more direct and easier to understand.
it('should not schedule next iteration if paused during callback', async () => {
const callback = vi.fn(async () => {
await new Promise((resolve) => setTimeout(resolve, 30));
});
const { pause } = useAsyncPolling(callback, { minInterval: 100 });
await vi.advanceTimersByTimeAsync(100);
expect(callback).toHaveBeenCalledTimes(1);
await vi.advanceTimersByTimeAsync(100);
expect(callback).toHaveBeenCalledTimes(2);
pause();
await vi.advanceTimersByTimeAsync(200);
expect(callback).toHaveBeenCalledTimes(2);
});| * - Set isActive to true | ||
| * - Clear lastError | ||
| * - Execute the callback immediately if immediateCallback is true | ||
| * - Schedule the next iteration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The phrase 'Schedule the next iteration' is a bit ambiguous. When resume() is called and immediateCallback is false, the current implementation schedules the first execution after waiting for the full minInterval. This might be surprising to a developer who expects polling to resume more quickly. It would be beneficial to make this behavior explicit in the documentation to improve clarity and prevent potential misuse.
| * - Schedule the next iteration | |
| * - Schedule the first iteration after `minInterval` if `immediateCallback` is false |
- Add pause function to callback options for easy access from within callback - No need to capture returned pause function separately - Allows conditional pausing based on callback results - Add 3 new tests verifying pause-in-callback functionality - Update documentation with example and explanation