Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Bug: Fixed missing headers #47

Merged
merged 1 commit into from
Mar 29, 2024
Merged

Conversation

Corepex
Copy link
Contributor

@Corepex Corepex commented Mar 22, 2024

Followup to: #46

Problem

It seems that the headers dont get passed to the request.

Fix

I created a fix for that but it should be considered as "quick and dirty".
IMHO there is a bigger problem with this part of the code generation. Also the names of the props are weird.

Copy link
Collaborator

@seriouslag seriouslag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this a bug? Does this solve an issue with the tests or fix the example?

Or should this change be added to the base request file?

@Corepex
Copy link
Contributor Author

Corepex commented Mar 22, 2024

@seriouslag - yeah this - or something smarter - should be added to the request file ...

It is a bug because "special headers" like Content-Type: application/merge-patch+json don't get passed from the generated service to the request.

In my case (for example) the PATCH method looks like that:
image

@seriouslag
Copy link
Collaborator

seriouslag commented Mar 22, 2024

I see your case. This library uses the package openapi-typescript-codegen for service generation. The default request file comes from that library. I am all for documenting your use case and fix in this package, but an issue/PR in that library may be better for the ecosystem. This PR changes the example code, not the code the end user will see. I can see how it would be helpful to have different request files in this library for examples of how to use this library.

@7nohe
Copy link
Owner

7nohe commented Mar 29, 2024

As @seriouslag said, the request file is not the responsibility of this library.
However, I think it makes a good example of how to customize headers, so I will accept this PR.

@7nohe 7nohe merged commit 4d97317 into 7nohe:main Mar 29, 2024
4 checks passed
@Corepex Corepex deleted the fix_missing_headers branch March 29, 2024 14:06
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants