ofetch icon indicating copy to clipboard operation
ofetch copied to clipboard

fix: normalize headers object before onRequest (nuxt/nuxt#27042)

Open cmgdragon opened this issue 1 year ago โ€ข 2 comments

๐Ÿ”— Linked issue

โ“ Type of change

  • [ ] ๐Ÿ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • [x] ๐Ÿž Bug fix (a non-breaking change that fixes an issue)
  • [ ] ๐Ÿ‘Œ Enhancement (improving an existing functionality like performance)
  • [ ] โœจ New feature (a non-breaking change that adds functionality)
  • [ ] ๐Ÿงน Chore (updates to the build process or auxiliary tools and libraries)
  • [ ] โš ๏ธ Breaking change (fix or feature that would cause existing functionality to change)

๐Ÿ“š Description

Resolves nuxt/nuxt#27042

It would be convenient to normalize the headers object before running the onRequest interceptor so there's no conflicts between the retried requests

๐Ÿ“ Checklist

  • [x] I have linked an issue or discussion.
  • [ ] I have updated the documentation accordingly.

cmgdragon avatar May 04 '24 23:05 cmgdragon

Looks reasonable to me! ๐Ÿ‘๐Ÿป

TheAlexLichter avatar May 05 '24 07:05 TheAlexLichter

Heads up! @manniL , I just realized, the current approach will only work either using the Headers constructor or setting the properties in lowercase. If we set a header in uppercase inside the onRequest, both properties will remain in the headers object and will be merged together after the retry.

For ensuring the headers object always keeps the same format and it isn't case-sensitive, we could add some more code after the intercerpor... Something like shown below ๐Ÿ‘‡

utils.ts

export function normalizeHeaders(headers: any) {
  if (headers instanceof Headers) {
    headers = Object.fromEntries(headers)
  }
}

fetch.ts

if (context.options.onRequest) {
  await context.options.onRequest(context);
}

// Normalize & merge headers
if (context.options.headers) {
  normalizeHeaders(context.options.headers);
  context.options.headers = {
    ..._options.headers ? normalizeHeaders(_options.headers) : {},
    ...Object.fromEntries(
      Object.entries(context.options.headers)
        .map(([k, v]) => [k.toLowerCase(), v])
    )
  };
}

But I don't know if we'd complicating the code too much with that, what do you think, should I commit it?

cmgdragon avatar May 05 '24 19:05 cmgdragon

Hi dear @cmgdragon i appreciate the attempt to fix but please first help to make a minimal reproduction of issue using ofetch alone and submit an issue report in this repo ๐Ÿ™๐Ÿผ (we might think to solve this differently after discussion)

pi0 avatar May 07 '24 11:05 pi0