Fix ExecuteOptions handling in ranking executeWithFields #210
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes ExecuteOptions handling in the ranking executeWithFields method and adds comprehensive test coverage. The changes enable passing custom fetch options (like headers) through the entire API call chain, from high-level builder methods down to the underlying fetch implementation.
Key changes:
- Introduced
ExecuteOptionstype withfetchOptionsproperty for passing RequestInit options - Updated
executeWithFieldsto accept ExecuteOptions through flexible overload signatures using a newresolveExecuteArgshelper method - Threaded ExecuteOptions parameter through all API execution methods (search, ranking, user search)
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/narou.ts | Added ExecuteOptions type definition and updated all execute methods to accept and forward options parameter |
| src/narou-fetch.ts | Updated execute method to pass ExecuteOptions.fetchOptions to the fetch call |
| src/narou-jsonp.ts | Updated execute method signature to accept ExecuteOptions (marked unused with void pattern since JSONP doesn't support fetch options) |
| src/ranking.ts | Added ExecuteOptions parameter to execute and executeWithFields methods; implemented resolveExecuteArgs helper to parse flexible argument patterns |
| src/search-builder.ts | Updated execute method to accept and forward ExecuteOptions |
| src/search-builder-r18.ts | Updated execute method to accept and forward ExecuteOptions |
| src/user-search.ts | Updated execute method to accept and forward ExecuteOptions |
| src/index.common.ts | Exported ExecuteOptions type for public API usage |
| src/index.ts | Fixed error handling in rankingHistory to throw error strings directly instead of wrapping in Error object |
| test/ranking.test.ts | Added comprehensive tests verifying ExecuteOptions propagation through executeWithFields calls |
| test/narou.test.ts | Added MSW setup for mocking API responses in integration tests |
| test/narou-fetch.test.ts | Added test verifying fetch options are properly passed to fetch implementation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // MEMO: このファイルのテストは外部APIを利用するため、結果が変わる可能性がある。 | ||
| // そのため、結果が変わる可能性が少ないものを選択してテストを行っているが、落ちるようになったら修正が必要。 | ||
| describe("narou-test", () => { | ||
| // まれに時間がかかるので30秒に設定 | ||
| vi.setConfig({ testTimeout: 30000 }); |
There was a problem hiding this comment.
このコメントは古くなっています。MSWを使用してAPIをモックするようになったため、テストは外部APIを利用せず、結果が変わることはありません。このコメントと、30秒のタイムアウト設定を削除するか、テストがモックを使用していることを反映するように更新してください。
Translation: This comment is outdated. Since the tests now use MSW to mock the APIs, they no longer use external APIs and the results will not change. Consider removing this comment and the 30-second timeout setting, or update them to reflect that the tests now use mocks.
Summary
Testing
Codex Task