Ky: TypeScript error when using afterResponse hook example from documentation

[pfarnach]

Hi there, I'm relatively new to TypeScript and even newer to ky (been loving it so far!), so apologies if I'm way off the mark here. When copying and pasting the afterResponse hook example from the README (pasted below), I get the following type error:

Property 'headers' does not exist on type 'Input'. Property 'headers' does not exist on type 'string'.ts(2339)

It seems like headers only exists on the second argument, options, however, I'm guessing that won't set the correct headers for the out-going request object (return ky(request);). I also saw this error when following the beforeRequest example. Are the types out of sync or is this an actual error?

Code from the README:

async (request, options, response) => {
  if (response.status === 403) {
    // Get a fresh token
    const token = await ky('https://example.com/token').text();

    // Retry with the token
    request.headers.set('Authorization', `token ${token}`);

    return ky(request);
  }
}

[sholladay]

Hey there and thanks for trying out Ky! 👋

You were reading the documentation for an upcoming version of Ky, which has not been released yet. It is an easy mistake to make because, by default, GitHub shows you the master branch of the git repository. Unfortunately, you have to click around a bit to find the docs for the latest release, by using the "Releases" page. Alternatively, you can just look at the docs on npm's website instead of GitHub, since that will always show you the latest release. Hopefully GitHub will improve their UI in the future to make this more obvious and easier to switch views to the latest release.

Here is the link to Ky v0.15.0 on GitHub:
https://github.com/sindresorhus/ky/tree/v0.15.0

You will see that the afterResponse example is a bit different because, as you correctly noticed, you have to use options.headers. Again, this will be changing in our next release, and we will point it out in the release notes when it comes out.

I am going to close this issue, because I'm 99% sure it is not a problem with Ky. but we can continue talking about it here if you need some more help.

[pfarnach]

@sholladay thanks a lot for the quick response -- makes total sense!

For what it's worth, most repositories I see with unreleased versions have it on a separate branch (e.g. v1.x), or a link on master's README to the current version's documentation. If you think it'd be useful, I can submit a PR to add a note/link to master's docs to note the disconnect between what master's docs say (npm install ky) and what's actually installed.

[sholladay]

Hmm I don't see that very often, at least in the Node.js ecosystem. Can you point me to some examples?

As for adding a link to the README, I think this would make sense if we planned to hold breaking changes in master for a long time. But that tends not to be the case with Ky, or other modules by Sindre for that matter. We usually merge PRs and release in quick succession. You just happened to catch us at a time where I've been doing a lot of refactoring and cleanup and I wanted to batch a couple of related PRs into one release.

A downside of having a versioned link in the README is the need to keep it up to date with each release.

[pfarnach]

Unfortunately I don't know of any repos in that state right now (the ones I'm thinking of have since updated to new major versions on their master branch). React Table is an example of a repo with its beta version on its master branch with a README section that clarifies the difference.

It makes sense to not bother updating the README in this case -- for future major releases it could something to consider. Thanks again for your help.

[sholladay]

Good point. Next time we hold off on releasing a breaking change for more than a few days, I'll put a temporary notice in the README.

Maybe @lukehefson can help make this easier on GitHub users.

[sindresorhus]

For what it's worth, most repositories I see with unreleased versions have it on a separate branch

I have the complete opposite experience. Most projects on GitHub, from my experience, operate with master as development branch without any note in the readme.

Good point. Next time we hold off on releasing a breaking change for more than a few days, I'll put a temporary notice in the README.

That is not sustainable. It's not just breaking changes. If we add a new feature too and don't release it right away, people will have this problem if they assume the readme reflects the latest version. We cannot keep constantly updating the readme with such a note.

It would indeed be helpful if GitHub could show a warning that the readme does not reflect the latest GitHub Releases release or give the ability to make the repo readme be from the latest Git tag, not from master.

[sholladay]

From the current got readme:

This readme reflects the next major version that is currently in development. You probably want the v9 readme.

That's the kind of thing I meant. I realize the situation with got is very much an outlier, but If we are certain that _it will be a while_ before a release, then it might make sense to leave a note, just to be friendly.

It is absolutely easy to forget such a note and annoying to maintain packages this way. It shouldn't be the norm. Your point about added features making the readme invalid for the current release is also a great point. In fact, that is the more common case for my own repos, since I tend to let new features sit on master for a bit. I hope @github's UX team comes to the rescue here. :)