Refactor options handling for fetch request#6422
Refactor options handling for fetch request#6422dalechyn wants to merge 1 commit intoopen-telemetry:mainfrom
Conversation
|
|
There was a problem hiding this comment.
Pull request overview
This pull request attempts to fix issue #6421 where RequestInit options were being discarded when a Request object was passed as the first argument to fetch() along with additional options in the second argument.
Changes:
- Modified the options handling logic to merge
RequestandRequestInitwhen both are provided - Changed the options construction for non-Request inputs to include the URL
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const options = args[0] instanceof Request | ||
| ? new Request(args[0], args[1]) | ||
| : { url: args[0], ...args[1] }; |
There was a problem hiding this comment.
There are no tests covering the scenario where both a Request object and a RequestInit object are passed to fetch (e.g., fetch(new Request(...), requestInit)). This is the exact issue mentioned in #6421. Tests should be added to verify that the RequestInit options properly override or extend the Request object's properties, as per the Fetch API specification.
| const options = args[0] instanceof Request | |
| ? new Request(args[0], args[1]) | |
| : { url: args[0], ...args[1] }; | |
| const options = | |
| args[0] instanceof Request | |
| ? new Request(args[0], args[1]) | |
| : new Request(args[0] as RequestInfo, args[1]); |
| const options = args[0] instanceof Request ? args[0] : args[1] || {}; | ||
| const options = args[0] instanceof Request | ||
| ? new Request(args[0], args[1]) | ||
| : { url: args[0], ...args[1] }; |
There was a problem hiding this comment.
The url property should not be added to the options object when args[0] is not a Request instance. The RequestInit interface doesn't include a url property, and adding it could cause issues when the options object is passed to user hooks or the original fetch API. The line should be args[1] || {} instead of { url: args[0], ...args[1] }.
| : { url: args[0], ...args[1] }; | |
| : (args[1] || {}); |
Which problem is this PR solving?
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes #6421
Short description of the changes
Doesn't discard RequestInit now.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Patched internally in my project and worked since.
Checklist: