Allow headers to be specified as functions.

[aaronschwartz]

I couldn't find a workaround for #171 without making a library change. This allows you to set a dynamic header value while setting up defaults. I considered making the entire headers object a function but that required many more changes that I didn't want to mess with since there are various places that it is treated specifically as an object.

E.g:

connect.defaults({
  headers: {
    "token": () => localStorage.getItem("token")
  }
})

[ryanbrainard]

Looks like a reasonable change, but looks like there's a test failure. Also, any thoughts on if this function should take any arguments? We can always add that later if a need arises, but just thinking about it.

[nfcampos]

why not use a getter?

connect.defaults({
  headers: {
    get token() { return localStorage.getItem("token") }
  }
})

[aaronschwartz]

I thought of using the getter as well but the object is spread into another object early on (at which point the getter is evaluated), but only the getter's value is passed on so it never gets evaluated again:
https://github.com/heroku/react-refetch/blob/master/src/components/connect.js#L45

I'll fix the Lint warning.

[nfcampos]

Fair enough, would it be better then to change the code to allow getters to be used, eg. using Object.getOwnPropertyDescriptors and Object.defineProperties instead of Object.assign?

[aaronschwartz]

I don't really mind either way as far as API goes. There are a couple of places where the headers are object.assign'd and merged with overrides. I was trying to do the least amount of changes possible so I didn't break anything. I'm not too familiar with getOwnPropertyDescriptors and defineProperties.

[nfcampos]

I tend to prefer to use language features in place of new APIs but don't mind too much either way.
Anyway, it's up to @ryanbrainard i'm just offering my 2 cents as someone very familiar with the library

[ryanbrainard]

@nfcampos Can you provide an example of what you're suggesting. If it's possible without changes to the lib, that sounds better, but I'm not totally following your suggestion.

[ryanbrainard]

@nfcampos Oh, now I see you did provide an example above in #172 (comment).

Given the amount of Object.assign sprinkled in this lib, I'm a little hesitant to change how a lot of that works, but that's probably mostly my own unfamiliarity with the ins and outs of getters, getOwnPropertyDescriptors, and defineProperties. How much work do you see it being to make this change and how risky would it be for backward compatibility?

Given that other areas of this library support the raw value or function pattern (like the prop mappings themselves), which probably could have also been done with getters, I'm leaning toward merging this PR as is because it will be familiar. Then later, we could consider also supporting getters, but doing it everywhere, not just headers. Thoughts?

[nfcampos]

@ryanbrainard I agree, this change should probably apply everywhere, not just to headers. And I'm afraid it should be a major version bump, because it does possibly change what values we use for properties in the mappers — ie. for users that were using getters we'd previously always be using the value the getter returned when we internally use Object.assign, and now we'd only invoke the getter once the value was actually used in a request.

The change would basically be replacing all usage of Object.assign with something like

function assign(target, ...sources) {
  for (source of sources.filter(Boolean)) {
    for (key of Object.keys(source)) {
      const descriptor = Object.getOwnPropertyDescriptor(source, key)
      if (descriptor.get) Object.defineProperty(target, key, descriptor)
      else target[key] = source[key]
    }
  }
  return target
}

So, it wouldn't be much work to change it, but it does sounds like something that would have to be considered more carefully.

[ryanbrainard]

Ok, sounds like that's something to save for a later day, but the way this is now will make headers work the same way other things do, so merging it.

[ryanbrainard]

Released in v1.0.1-1 (beta)

[ryanbrainard]

Released in v1.0.1