-
-
Notifications
You must be signed in to change notification settings - Fork 380
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
fix(fetch): pass headers to fetch
when headers
option is activated
#1780
Conversation
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.
LGTM but i will let @soartec-lab review
Sure, please give me a few days to review it 👍 |
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.
@clemeth
Thanks!
Since we are managing test cases, could you please add a test case that uses headers: true
?
https://github.com/orval-labs/orval/blob/master/tests/configs/fetch.config.ts
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.
Thanks!
Hi @clemeth
|
Hello, @camillecroci. This PR actually introduced a bug which was fixed by #1770 before the next release, so please refer to that for current code. I can still try to clarify some things. The const result = await getFoo({ "User-ID": userId }); With the option disabled, headers have to be passed through the const result = await getFoo({ headers: { "User-ID": userId } }); It's unclear to me what you're looking for, but this doesn't seem like it. Can you share some more details? |
aaah, yes it is very different indeed !
The return type:
It used to have only |
Could it be because of your custom |
the customFetch function has not changed since those |
Oh, of course, I was too quick there. That behavior was changed in #1699, I believe. There is, however, the We are getting off topic here, so perhaps you could open a feature request if that is something you need. |
@clemeth thank you very much for finding that out. I will open a feature request then ! |
Status
READY
Description
Passes headers to
fetch
whenheaders
option is activated.Fixes #1779.
I have not tested the patch since the build system doesn't support the system I am on.