Got v10 beta feedback

[sindresorhus]

This issue is for discussing the upcoming Got v10 version which is now in alpha beta.

Beta 1: https://github.com/sindresorhus/got/releases/tag/v10.0.0-beta.1

Let us know if you encounter any issues with upgrading or if anything is unclear in the release notes.

[gzaripov]

I tried got@10 and I stumbled upon this error
'url' must not begin with a slash when using 'prefixUrl'.
It looks weird for me, it is common use case when I make a Transport abstraction in my app and use it in many places with relative urls. And it is important to have leading slash because it gives reader knowledge that this is a relative url.

What are cons of allowing leading slash when using prefixUrl?

[arcanis]

This is not a Got issue. Got uses the Node.js 10 API, and some agents don't support it - they overwrite http.request and https.request with only two arguments: options and callback, while Got provides url, options and callback.

Fwiw I got this error with got.post, and reverting to 9 worked. I don't use agents.

[szmarczak]

@arcanis A runkit example would be lovely.

[arcanis]

It doesn't repro on runkit. After investigating a bit it seems to be related to Jest, although I don't know if the bug comes from Jest or Got. Repro:

yarn init -y
yarn add jest got

foo.js

const got = require(`got`);
got.post(`https://httpbin.org`, {
    responseType: `json`,
    json: {}
});

foo.test.js

test(`foo`, () => {
    require(`./foo`);
});

Running with node ./foo.js yields an HTTP 405 (which is ok), whereas running yarn jest yields the error aforementioned ("The "listener" argument must be of type Function").

[gajus]

In all of the reported cases, it is safe to assume that something in your dependency chain is overriding http.request. Given that this is such a common problem, I think it is very stubborn of Got maintainers not to recognize that it is a backwards incompatibility issue.

[Mathspy]

v10 is a major version bump (AKA potential backwards incompatible) and @sindresorhus mentioned that it might be a good idea to document incomparable agents/libraries
I don't think there's much the library maintainers can do in this situation besides document incompatible libraries/agents/frameworks

[gajus]

I don't think there's much the library maintainers can do in this situation besides document incompatible libraries/agents/frameworks

It is literally couple of lines of code to make it backwards compatible. The only reason it is not done, is because Got v10 intentionally drops < Node.js v10 support.

[szmarczak]

Also we wanted to drop legacy Url support, hence the URL instance (options.url).

[Mathspy]

That's an explicit decision from the maintainers that has several clear advantages like less maintenance burden, as well as stability on got v10 for the foreseeable future (instead of being forced to release v11 when they eventually decide to drop support for node v8)
Plus node v8's LTS is dropping at the end of 2019 so there isn't much of a reason to remain compatible with it

[gajus]

Sure, it is just that this will result in one of the two scenarios:

1. People are not going to update to Got v10, because most of the applications will have a dependency in their tree that breaks Got.
2. Someone is going to kick off a fork of got.

[szmarczak]

People are not going to update to Got v10, because most of the applications will have a dependency in their tree that breaks Got.

That's their responsibility to be up to date with Got.

[szmarczak]

The first alpha release was in September so there was plenty of time to adjust the code.

[yovanoc]

@szmarczak Yeah you're right but I know what he want to say, it can be hard in a project with multiple dependencies to tell each author to update their codes

[szmarczak]

You can fix this using a beforeRequest hook. Just normalize options.url using urlToOptions and merge the properties into options.

[Mathspy]

1. People are not going to update to Got v10, because most of the applications will have a dependency in their tree that breaks Got.
2. Someone is going to kick off a fork of got.

I don't see either of these as a bad thing, it's totally okay to rely on got v9 for few more months until dependencies have updated, and during that time it'd be great to open PRs against those dependencies making them compatible!
And it's totally fine for someone to fork got temporary! The maintenance burden falls back on them and they can keep the fork until eventually dependencies have all updated and the fork's goal no longer exists (though if they have a reason to keep it longer, its their burden and its their freedom, of course!)

[djpate]

You can fix this using a beforeRequest hook. Just normalize options.url using urlToOptions and merge the properties into options.

Can you elaborate? Trying to use this with serverless and I've been banging my head

[szmarczak]

Can you elaborate?

Umm... It's plain English, but here's the same sentence in Node.js:

https://runkit.com/szmarczak/5e1a212ef1a19c001a0e5fce

[szmarczak]

Updated that runkit example (it was broken because I was working on the second solution and didn't notice that it was the first runkit sheet). Note that it doesn't pass request as a second argument, but a third.

[szmarczak]

@arcanis @djpate Here's the solution which should work, it uses options.request: https://runkit.com/szmarczak/5e1a23d8e4d193001a709110

[djpate]

@szmarczak Thanks for the code.

[szmarczak]

You may be wondering why we don't do that: it's because we want to get rid of urlToOptions to remove legacy URL usage completely.