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

Fix fetchJSON adds useless Content-Type header for empty requests #9613

Merged
merged 5 commits into from
Jan 25, 2024
Merged

Fix fetchJSON adds useless Content-Type header for empty requests #9613

merged 5 commits into from
Jan 25, 2024

Conversation

chrisDeFouRire
Copy link
Contributor

I'm using Fastify on the backend, and it refuses DELETE requests sent from react-admin's ra-data-json-server.
It is because fetchUtils.fetchJson is setting the Content-Type header to application/json, even with an empty body like in the delete requests. The function that is setting the header is createHeadersFromOptions in fetch.ts.

It is not against the RFCs to do so, but it just breaks Fastify, and it makes little sense to send a Content-Type with a body that doesn't adhere to the content type: an empty body isn't valid json.

This pull request fixes the issue and adds new unit tests to ensure there is no regression.

yarn test-unit -- ./packages/ra-core/src/dataProvider/fetch.spec.ts passes.

I guess the alternatives are:

  • sending a body if there isn't one already (with 'null' in it)
  • modifying the ra-data-json-server package (and other data providers which are using DELETE with no body) to specify a body.

I thought fixing the createHeadersFromOptions option was the cleanest

@chrisDeFouRire chrisDeFouRire changed the title Sending json Content-Type header for empty body breaks some parsers Sending json Content-Type header with an empty body breaks some parsers Jan 25, 2024
Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

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

Nice!

packages/ra-core/src/dataProvider/fetch.ts Outdated Show resolved Hide resolved
packages/ra-core/src/dataProvider/fetch.spec.ts Outdated Show resolved Hide resolved
@chrisDeFouRire
Copy link
Contributor Author

chrisDeFouRire commented Jan 25, 2024

Thank you for giving us React Admin! it's a great tool ! and seeing how easily I can contribute makes me think it's the perfect tool for us even more! 🙏🙏

@fzaninotto fzaninotto changed the title Sending json Content-Type header with an empty body breaks some parsers Fix fetchJSON adds useless Content-Type header for empty requests Jan 25, 2024
@fzaninotto fzaninotto merged commit 90e3fab into marmelab:master Jan 25, 2024
10 checks passed
@fzaninotto fzaninotto added this to the 4.16.8 milestone Jan 25, 2024
@fzaninotto
Copy link
Member

Thanks for your PR, and for your kind words!

@chrisDeFouRire chrisDeFouRire deleted the fix-empty-body-fetch branch January 25, 2024 12:47
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.

2 participants