Got: Got v9

[sindresorhus]

Probably for version 9 at some point in the future. (This is not happening soon)

• Target Node.js 8
• Use async/await, destructuring, spread
• Use require('url').URL (WHATWG URL) over require('url') and require('url').URLSearchParams over require('querystring')
• Remove safe-buffer dependency
• Reduce dependencies

Ideas welcome for breaking changes. New features and bugs should be opened as individual issues.

[brandon93s]

• Use util.promisify over pify
• Replace is-plain-obj dependency with is and leverage throughout for additional readability and code clarity: (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, etc)
• Use require('url').URL (WHATWG URL) as a replacement for isurl and url-to-options dependencies
• Extract errors into separate file and use an assignment syntax similar to http methods (e.g. for (error of errors))

[sindresorhus]

Replace is-plain-obj dependency with is and leverage throughout for additional readability and code clarity: (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, etc)

Extract errors into separate file and use an assignment syntax similar to http methods (e.g. for (error of errors))

We can do these today. No need to wait. PR welcome.

[brandon93s]

I'll tackle the is implementation once https://github.com/sindresorhus/is/pull/34 lands!

[brandon93s]

@sindresorhus @lukechilds - Now that 8.x is LTS, what timeline would you guys like to target for switching over to targeting 8?

Version Metrics

Edit: I'm guessing we'd be looking to target 6 as a minimum version next, not 8?

[lukechilds]

My understanding from the .travis.yml was that all maintained LTS releases are supported. Is that correct @sindresorhus?

If so, when 4.x reaches EOL (end of this month) we'll drop support.

And when 10.x is released (sometime this month) we'll add support.

Edit: I'm guessing we'd be looking to target 6 as a minimum version next, not 8?

Yeah, at some point this month we should be supporting:

node_js:
  - '10'
  - '8'
  - '6'

[sindresorhus]

I plan to target Node.js 8

[lukechilds]

And drop support for 6? Great.

Object spread all the things!

[sindresorhus]

Yup

[sindresorhus]

I've started the work on this (https://github.com/sindresorhus/got/commit/2b1453734a0b51e5b5663b29c258a831dfe926f8). Let me know if anyone wants to help out.

[sindresorhus]

Use require('url').URL (WHATWG URL) as a replacement for isurl and url-to-options dependencies

@brandon93s I've replaced isurl now, but not sure how we can replace url-to-options as it's needed so we get an object to pass to http.request(). What did you have in mind?

[szmarczak]

@sindresorhus I want to help :unicorn:

[sindresorhus]

@szmarczak That would be amazing! I could definitely use some help. I'm trying to get out a new major release now and want to fix as many issues as possible, especially things that require a breaking change.

[sindresorhus]

Join me in https://gitter.im/got-dev/Lobby if you want to chat. I'll go to sleep soon, but I'll be back there tomorrow.

[szmarczak]

@sindresorhus It's not needed to pass an object to http.request(). See https://nodejs.org/dist/latest-v8.x/docs/api/http.html#http_http_request_options_callback

options \

url-to-options is a bit outdated. The newer version is:

https://github.com/nodejs/node/blob/8476053c132fd9613aab547aba165190f8064254/lib/internal/url.js#L1321-L1340

Instead:
https://github.com/sindresorhus/got/blob/09eee393d71e5810c93a5cf62150f95798a0e21c/index.js#L537-L542

We can do something like this:

 options = {
    url,
    ...options
 };

I'll try to do that :fire:

[sindresorhus]

url-to-options is a bit outdated. The newer version is:

Yup, I've already changed to bundling it here instead.

[sindresorhus]

It's not needed to pass an object to http.request().

But there's no way to pass an URL and options to http.request... You have to choose either.

[szmarczak]

But there's no way to pass an URL and options to http.request... You have to choose either.

Oh, you're right. I forgot (headers etc.). :confused: I think we can't get rid of that then.

[brandon93s]

@brandon93s I've replaced isurl now, but not sure how we can replace url-to-options as it's needed so we get an object to pass to http.request(). What did you have in mind?

Reviewed again, not sure what I was thinking. Appears necessary.

[millette]

Meanwhile, #526 (link to current docs) might be helpful.

[dertieran]

Hey, not sure if this is the right place to give, I can also move this into a separate issue.
After playing around with got.extend for building API wrappers, I ran into a problem with the stream and json option.

Let's say I want to build a wrapper arround jsonplaceholder my normal use case would be to set the json option by default

const placeholder = got.extend({
  baseUrl: 'https://jsonplaceholder.typicode.com',
  json: true,
});

const { body: posts } = await placeholder('posts');

But now I get an error when using placeholder.stream('posts')

Got can not be used as a stream when the `json` option is used

My solution for now felt a bit hacky and I guess I wouldn't be the only one who needs to do something like this

const placeholder = got.extend({
  baseUrl: 'https://jsonplaceholder.typicode.com',
  hooks: {
    beforeRequest: [
      options => {
        options.json = !options.stream;
      },
    ],
  },
});

Could something like this be done by default?
Or is there a better solution for this already?

[szmarczak]

@DerTieran

Could something like this be done by default?

It could be, but we need to discuss this. If it's the most what users want, then I think yes.

Or is there a better solution for this already?

No.

[sindresorhus]

I think we should handle this somehow. It seems like a common need and the workaround is ugly. How should we handle it though? We could ignore the json option when it's a stream and it was created by extend/create, and document it? Any other suggestions?

[szmarczak]

We could ignore the json option when it's a stream and it was created by extend/create, and document it?

:+1:

Any other suggestions?

I don't see any. The solution above would be best IMO.

[sindresorhus]

Alright. Let's do that then.

[dertieran]

Any other suggestions?

I just had another idea, because I needed to use a API where I need to encode the body as form and decode as json.
(Often used in OAuth2 when exchangin a code for a token)

got.post('unicorn.com/oauth/token', {
  form: true,
  json: true,
  body: { code: 'rainbows' },
})

For me this was always hard to read and needed an understanding how got handles this special case.

Why not just add a encode/decode option or something similar?
This way the stream problem could be generalized to only allow encode or ignore decode.
encode might be confusing because there is already encoding

So it might look like this

got.post('unicorn.com/oauth/token', {
  encode: 'form',
  decode: 'json',
  body: { code: 'rainbows' },
})

Maybe there are better names for these option or I might be alone with this problem.

[szmarczak]

Similar ideas were proposed many, many times. I don't think it's worth it, because to achieve the same you just need one more line of code: let parsed = JSON.parse(response.body);

[sindresorhus]

Got 9 is out now: https://github.com/sindresorhus/got/releases/tag/v9.0.0 ✨