Skip to content
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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants