Improve polling#89
Conversation
…able in response object already)
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the polling mechanism to allow finer control over how and when polling executes. Key changes include:
- Introducing new polling configuration options (pollingDelay and maxPollingAttempts).
- Refactoring polling logic into a separate withPolling function.
- Updating tests and documentation to reflect these changes.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/request-handler.spec.ts | Updates to timer usage and tests for polling behavior. |
| src/types/request-handler.ts | Adds new pollingDelay and maxPollingAttempts options in type defs. |
| src/request-handler.ts | Refactors polling logic by delegating to the new withPolling helper. |
| src/polling-handler.ts | Introduces the withPolling function to manage polling retries. |
| README.md | Updates documentation with examples for the new polling options. |
Comments suppressed due to low confidence (1)
src/request-handler.ts:370
- The new pollingDelay option is defined in the types and documentation but not utilized here. Consider passing mergedConfig.pollingDelay to withPolling (or incorporating it within the polling logic) to apply the intended delay before each polling attempt.
pollingInterval > 0 ? withPolling<FetchResponse<ResponseData, RequestBody, QueryParams, PathParams>>(doRequestOnce, pollingInterval, mergedConfig.shouldStopPolling, mergedConfig.maxPollingAttempts)
size-limit report 📦
|
There was a problem hiding this comment.
Pull Request Overview
This PR centralizes and enhances polling behavior in the request handler by introducing new configuration options, moving polling logic into its own utility, and updating tests and documentation.
- Added
pollingDelayandmaxPollingAttemptsoptions and removed the unused error parameter fromshouldStopPolling. - Refactored polling into
withPollingand updatedcreateRequestHandlerto use it. - Expanded unit tests for polling and refreshed documentation and examples to reflect new options.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/request-handler.spec.ts | Adjusted timer setup and added infinite‐loop protection test. |
| test/polling-handler.spec.ts | New unit tests covering withPolling scenarios. |
| src/types/request-handler.ts | Updated PollingFunction type and added pollingDelay/maxPollingAttempts. |
| src/request-handler.ts | Refactored to call withPolling instead of inline polling logic. |
| src/polling-handler.ts | Introduced new withPolling function. |
| package.json | Expanded lint scope to include example files. |
| docs/examples/examples.ts | Added example9 demonstrating polling. |
| README.md | Expanded polling section with new options and behaviors. |
Comments suppressed due to low confidence (1)
test/request-handler.spec.ts:375
- This test asserts on
delayInvocationbut it isn’t mocked or spied on, so the expectation will always fail. You should spy on or mockdelayInvocation(e.g. usingjest.spyOn) or adjust the assertion to reference the actual mock.
expect(delayInvocation).toHaveBeenCalledTimes(2);
There was a problem hiding this comment.
Pull Request Overview
This pull request improves the polling functionality by adding new configuration options, refactoring the polling logic into a dedicated function, and updating documentation and tests to reflect these changes.
- New polling options (pollingDelay, maxPollingAttempts) have been added and incorporated in the polling handler.
- The polling logic has been refactored into a new withPolling function, and tests have been updated to validate these changes.
- The documentation and examples have been expanded to explain the new polling options and their usage.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/request-handler.spec.ts | Updated tests for polling behavior; note possible inconsistency in delay mock reference. |
| test/polling-handler.spec.ts | Added comprehensive tests for the new withPolling function. |
| src/types/request-handler.ts | Updated type definitions to remove the error parameter in shouldStopPolling. |
| src/request-handler.ts | Refactored polling logic to use withPolling and updated deduplication handling. |
| src/polling-handler.ts | Introduced withPolling function for centralized polling logic. |
| package.json | Expanded linting scope to include example files. |
| docs/examples/examples.ts | Added example9 to demonstrate new polling options. |
| README.md | Updated documentation to include new polling options and usage. |
Comments suppressed due to low confidence (2)
README.md:708
- The documentation still mentions an 'error' parameter for shouldStopPolling, even though the updated signature accepts only (response, attempt). Please update the description to remove any reference to the error parameter.
A function to determine if polling should stop based on the response, error, or the current polling attempt number.
test/request-handler.spec.ts:375
- There appears to be an inconsistency with the delay function mock: earlier the test referenced 'mockDelayInvocation' while here it uses 'delayInvocation'. Please ensure the tests consistently refer to the correct mock for clarity.
expect(delayInvocation).toHaveBeenCalledTimes(2);
This pull request introduces a comprehensive overhaul of the polling functionality in the codebase. Key changes include the addition of new polling configurations, refactoring of polling logic to improve modularity and maintainability, and the introduction of unit tests to ensure robust behavior. These updates enhance flexibility, allow finer control over polling behavior, and improve code readability.
Polling Configuration Enhancements:
pollingDelayandmaxPollingAttemptsto the polling configuration, allowing a delay before each polling attempt and limiting the number of attempts. Updated theREADME.mdto document these options. [1] [2] [3]Refactoring of Polling Logic:
withPollingfunction insrc/polling-handler.tsto encapsulate polling logic, making it reusable and modular. This function supports polling intervals, delays, and maximum attempts.createRequestHandlerinsrc/request-handler.tsto delegate polling logic towithPolling, simplifying the request handler implementation. Removed redundant polling-related code. [1] [2] [3] [4] [5] [6]Type Updates:
PollingFunctiontype andExtendedRequestConfiginterface insrc/types/request-handler.tsto reflect the new polling options and ensure compatibility with the refactored logic. [1] [2]Unit Tests for Polling:
withPollingintest/polling-handler.spec.tsto validate polling behavior, including scenarios for delays, maximum attempts, and custom stop conditions.createRequestHandlerintest/request-handler.spec.tsto verify integration with the new polling functionality. [1] [2] [3]Auxiliary Changes:
package.jsonto include example files for better code quality enforcement.example9) indocs/examples/examples.tsto demonstrate usage of the updated polling options. [1] [2]