unsplash / unsplash-js

🤖 Official JavaScript wrapper for the Unsplash API
https://www.npmjs.com/package/unsplash-js
MIT License
2.15k stars 157 forks source link

Spreading headers may lose data #188

Open OliverJAsh opened 2 years ago

OliverJAsh commented 2 years ago

Related: https://github.com/unsplash/unsplash-web/pull/7397

https://github.com/unsplash/unsplash-js/blob/07050e9d8d7d3655e7ca1981a8db8a827a3c6745/src/helpers/request.ts#L37-L40

The type of the headers variable that we're spreading here is HeadersInit | undefined.

HeadersInit is defined as:

type HeadersInit = string[][] | Record<string, string> | Headers;

What happens when we merge an array (string[][]) into an object? The array indexes become header keys. That's not the correct behaviour—we should instead use the key inside the entries array.

const a = [['a', '1']]
const b = { ...a }
JSON.stringify(b)
// => '{"0":["a","1"]}'

Desired result: { a: 1 }

What happens when we merge a Headers class into an object? We lose all data!

const a = new Headers([['a', '1']])
const b = { ...a }
JSON.stringify(b)
// => '{}'

Desired result: { a: 1 }

Related: a lint rule could have caught this https://github.com/typescript-eslint/typescript-eslint/issues/748