fix: should handle empty content (HTTP 204)

[KotoriK]

Changes

It won't throw error while handling response with no content, which is a common practice for DELETE method. Instead it will return empty in data or error.

Checklist

• Tests updated
• README updated (no need to change? )

[drwpow]

Thank you!

I wasn’t able to find any performance concerns with replacing res.json() with JSON.parse(await res.text())—they both seem to perform just as fast in both Firefox and Chrome. Thanks!

[KotoriK]

Thank you!

I wasn’t able to find any performance concerns with replacing res.json() with JSON.parse(await res.text())—they both seem to perform just as fast in both Firefox and Chrome. Thanks!

The only concern I used Response.text() here is to check the length of the response's body.
However I have just recalled that by parsing headers, we can also get knowledge of the length of the content. So usingResponse.json() is probably fine.
Also I recall that I forgot to handle JSON parsing error. Fallback to string might be a proper solution, or check Content-Type before parsing it.

[drwpow]

However I have just recalled that by parsing headers, we can also get knowledge of the length of the content.

That’s true—I think that’s probably a better way to handle empty content.

Also I recall that I forgot to handle JSON parsing error.

That’s also fine to not tackle in this PR. For now I actually think it’s preferable this library does NOT catch errors, as that should bubble up to the parent function calling this. If this library tries to self-rescue, it gets into the realm of opinion (and limits its usage)

[drwpow]

Hm it still seems like lint & tests aren’t passing. If you have Prettier installed globally, it’s better to uninstall it and use the local copy. Also be sure to use the latest version of Node (18.x or 19.x) and re-install all dependencies with pnpm i.

Also be sure to run npm test locally too to make sure it’s passing on your local machine.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 9c1d6c6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[KotoriK]

Sorry, it will be fine now.

[drwpow]

Also, sorry—I didn’t realize #28 conflicted with this PR. Both PRs are needed! Checking the status length is important to do as well as the status code (which can be a shortcut). Apologies for merging that one first; it was accidental.