americanexpress / fetchye

✨ If you know how to use Fetch, you know how to use Fetchye [fetch-yae]. Simple React Hooks, Centralized Cache, Infinitely Extensible.
Apache License 2.0
41 stars 21 forks source link

feat(fetchye): http headers casing does not affect cache key creation #53

Closed Jimchapter closed 2 years ago

Jimchapter commented 2 years ago

Making http header names case insensitive for cache key creation.

Description

If headers are provided in the options object, making all the header names lowercase as to not generate a different cache key for the same names but different casings. For example Content-Type, content-type or CONTENT_TYPE will all be treate d the same way during cache key creation.

Motivation and Context

HTTP headers let the client and the server pass additional information with an HTTP request or response. An HTTP header consists of its case-insensitive name followed by a colon (:), then by its value. Whitespace before the value is ignored.

How Has This Been Tested?

Wrote unit tests for this that assert that the cache key created for the same header names, but different casings are equal. Also functionally tested this locally in an AmericanExpress intranet app where I am using the fetchye framework.

Types of Changes

Checklist:

What is the Impact to Developers Using Fetchye?

No direct impact to developers unless maybe someone has written their own mapOptionsToKey function for their fetchyes and is relying on header name case sensitivity.

codesandbox-ci[bot] commented 2 years ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 7371a86c87d9674f6d6dc9a37676194030810f33:

Sandbox Source
quick-install Configuration
fetchye-provider-install Configuration
fetchye-redux-provider-install Configuration
nextjs-fetchye-ssr Configuration
Matthew-Mallimo commented 2 years ago

Could this be considered a breaking change ?

Jimchapter commented 2 years ago

Could this be considered a breaking change ?

My argument against that is that this has no impact to functionality unless someone was specifically relying on different casings for header names to have their own cache entry.

I actually think it might classify more as a bugfix as per the spec header names are case insensitive, but without this change there are cache misses for the same requests with different casings in the header names.

I committed as a new feature, however, as this is how it's mentioned in the user story.