chore(gatsby-source-filesystem): update got dependency

[EnKrypt]

Description

Using gatsby-source-contentful with certain options triggers a particular DeprecationWarning in node v12. (For more info, follow issue link below)

Bumping got to 9.6.0 in gatsby-source-filesystem fixes this.

The only side-effect is that this requires min Node v8. I don't think that should be a problem, but feel free to correct me.

Checked with all tests passing.

I'm not sure what the protocol is following this, but I'm guessing after this is merged, it will be reflected when another PR with chore(release) bumps all depending package versions to publish.

Related Issues

Fixes #18433

[pieh]

got@9.6 requires node@>=8.6 ( https://github.com/sindresorhus/got/blob/v9.6.0/package.json#L8-L10 ) and our packages declare support for node>=8.0 ( https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-source-filesystem/package.json#L52-L54 ), so this is tricky, because we would have to drop support for >=8.0 && <8.6 node versions with this change.

Depending when this was changed in got we might be able to bump maybe to got@8 which still supports older node versions. If this was fixed in got@9 then there's pickle - either live with deprecation warnings or drop support for some node versions (which should be considered breaking change)

[EnKrypt]

So I went through the releases while debugging this.

Unfortunately the first release of got that fixes this is got@9.0.0 which does require node@>=8.6.

Now, the actual fix within got itself is trivial. Pre 9.0.0, it depends on timed-out@4 which is what throws the DeprecationWarning. From got@9.0.0, it no longer depends on that package, but timed-out@5 fixes the issue as well.

I initially thought we could keep the old version of got but bump timed-out in the package lockfile, but sadly we're not adding lockfiles to version control. I tried starting a conversation about this in the Discord, but haven't had any replies yet.

What do you suggest?

[EnKrypt]

Btw, I see from the existing lockfile that we are already depending on got@^9.6.0. How does that play out then?

We can also alternatively just remove the dependency on got outright. The package makes only one call to it (for downloading a remote file), and that can easily be replaced by something else.

[pieh]

Now, the actual fix within got itself is trivial. Pre 9.0.0, it depends on timed-out@4 which is what throws the DeprecationWarning. From got@9.0.0, it no longer depends on that package, but timed-out@5 fixes the issue as well.

I initially thought we could keep the old version of got but bump timed-out in the package lockfile, but sadly we're not adding lockfiles to version control. I tried starting a conversation about this in the Discord, but haven't had any replies yet.

We can't really mess with dependencies with packages we don't control. Yarn has some support for this ( https://yarnpkg.com/lang/en/docs/selective-version-resolutions/ ), but npm doesn't support it.

I was checking combination of npm aliases and optional dependencies, but npm added support to package aliases in npm@6.9 and versions before that doesn't understand the syntax and refuse to install, so this is no go.

Btw, I see from the existing lockfile that we are already depending on got@^9.6.0. How does that play out then?

Yeah, you are right.

We already use got@9 with gatsby-source-contentful, because is-online package that this plugin uses, depends on got@9 ( https://unpkg.com/browse/is-online@8.2.0/package.json ).

Maybe doing the swap is ok, and it won't disturb the users. I looked up our telemetry stats, and seems like over last month people that run gatsby on Node@>=8.0 && < 8.6 were 0.01% of our userbase. It might be ok to go forward with this, given that we already broke >=8.0 support with contentful plugin itself, but I do need to sleep on it a bit to make decision like that.

We can also alternatively just remove the dependency on got outright. The package makes only one call to it (for downloading a remote file), and that can easily be replaced by something else.

Sure, but it also abstract few things that we don't have to handle ourselves. I would be wary of removing it and re-implementing things we need.

[EnKrypt]

Maybe doing the swap is ok, and it won't disturb the users. I looked up our telemetry stats, and seems like over last month people that run gatsby on Node@>=8.0 && < 8.6 were 0.01% of our userbase. It might be ok to go forward with this, given that we already broke >=8.0 support with contentful plugin itself, but I do need to sleep on it a bit to make decision like that.

Sure thing. Let me know what you decide is the best way forward. I'll keep this PR open till then.

[Lemraj]

I am having the same problem when running http-server (local).

I have a simple angular app:
1 - ng build --prod
2 - go to dist/{name-app}
3 - run http-server

when browsing the url i get the following error:

[EnKrypt]

@Lemraj I see that you've encountered the same DeprecationWarning, but I admit I don't quite see how it might relate to this PR or Gatsby.

Am I correct in assuming that you happened to land here while searching for this issue?

In that case, this issue in http-server might be more relevant.

[Lemraj]

@EnKrypt You are right! i was searching for this issue and landed here!
I will post my question in the http-server issues page. Thanks.

[pieh]

Ok, I think the best move (still not great, but best we can do) is to not merge that yet:
It's unfortunate that we broke node version in gatsby-source-contentful (this should be considered as bug), but this plugin is used far less than gatsby-source-filesystem (which is used pretty much with almost every gatsby site).

Given that this PR gets rid of deprecation warning (and doesn't fix something that is broken), I think it's fair not to make the change yet. So upside of the change is that users on node 12 don't get warning, downside is that users on officially supported >=8.0 <8.6 will have problems with using this package (depending on package manager this can mean either not being able to install the package, to potential build time problems)

We will be dropping Node 8 support when it will reach its "end of life" which will happen in ~2 months ( https://nodejs.org/en/about/releases/ ), so that will be great time to make this change.

Any thoughts on that? If we can agree on this I would close PR for now, and this can be reoponed on 1st of January 2020, as this would be when we would officially drop node 8 support

[EnKrypt]

I think you've made a reasonable argument.

Alright! Feel free to close this.

Now I just have to make sure I'm not too drunk or hungover on New Year's to remember to re-open this issue. Quite the challenge, but I'm up for it.

[pieh]

Now I just have to make sure I'm not too drunk or hungover on New Year's to remember to re-open this issue. Quite the challenge, but I'm up for it.

Hahaha, I didn't mean to re-open precisely on 1st of January 2020 ;) We will also probably need few days to get rest of our stuff in order (update all engines declaration in all of our packages, docs etc)

[EnKrypt]

@pieh Re-opened as discussed. If we've dropped support for node 8, I think we should be good to go.

[DanielRuf]

packages/gatsby-source-filesystem/package.json

Can you take a look the conflict and resolve it?

[sidharthachatterjee]

We're adding a deprecation warning in #20466 and delaying breaking support for another month or so

[EnKrypt]

Any updates? What are the current stats on our users using Node < 10.13?

[sidharthachatterjee]

Time to get this in!

[AleC77]

Hi, with gatsby 3.0 I am seeing again the same problem. Any suggestion?

(node:18172) [DEP0066] DeprecationWarning: OutgoingMessage.prototype._headers is deprecated
at module.exports (node_modules\timed-out\index.js:9:17)
at EventEmitter. (node_modules\got\index.js:244:5)
at Object.onceWrapper (events.js:313:26)
at EventEmitter.emit (events.js:223:5)
at makeRequest (node_modules\got\node_modules\cacheable-request\src\index.js:94:9)
at node_modules\got\node_modules\cacheable-request\src\index.js:104:14
at runNextTicks (internal/process/task_queues.js:59:5)
at processImmediate (internal/timers.js:412:9)

[qubis741]

Same with gatsby 3.1.2

[zackthoutt]

This happened to me as well. Fixed it by adding the latest version of got to my dependencies.

"got": "^11.8.2"