Deprecate Electron support

[szmarczak]

Fixes #899
Fixes #990

Checklist

• I have read the documentation.
• I have included a pull request description of my changes.
• I have included some tests. (cannot run ava + electron yet)
• If it's a new feature, I have included documentation updates.

[sindresorhus]

Can you move the changes that fixes #990 into a separate PR?

[sindresorhus]

Can you remove the added Electron workarounds? There's no point in fixing Electron issues now that we're deprecating support for it.

[szmarczak]

Can you move the changes that fixes #990 into a separate PR?

Will do, I just did so because without these changes Electron support is fully broken though.

[szmarczak]

Can you remove the added Electron workarounds?

All the other fixes are required except the redirect fix. It's only 55 lines long. Should I remove it?

[szmarczak]

Some other unnecessary fixes:

• session uses the value from electronSession (by default it's a TLS session, not an electron one) - 4 lines
• it automatically throws if no auth info provided (the promise would never resolve otherwise) - 10 lines

The content-length header fix is required as without it POST requests would not be possible anymore.

[sindresorhus]

It’s not about the amount of lines, but rather that everything adds complexity, and we have a lot of complexity already. Doesn’t really make sense to add complexity for something we intend to remove. These fixes might even cause problems to other things.

[szmarczak]

Summary:

The last known Got version that was Electron-compatible was v9.6.0. At the time the Electron version was v3.1.1. A few days ago v7.1.5 has been released.

electron/electron@2a37934 kicked in and broke reading error responses.
electron/electron@b7defaa kicked in and broke sending CORS headers.
So, Got v9.6.0 has been no longer compatible with Electron v7.0.0 and up,
which has been released on October 22 - about two months ago.

[szmarczak]

These fixes might even cause problems to other things.

I don't think so, I've tested it locally. Anyway, I'll remove those fixes :)

People who switched to Electron 7 two months ago or switched to Got 10 this month - had to disable useElectronNet, there was no other solution. But yeah, not everybody keeps their dependencies up-to-date :P

[szmarczak]

Can you move the changes that fixes #990 into a separate PR?

Is that really needed? It's not so complicated. Basically the fix is this: https://github.com/sindresorhus/got/pull/995/files#diff-7bf54112e60a8714e49d3e1e98cf4e2bR32-R36

[sindresorhus]

Is that really needed? It's not so complicated.

It's not about how complicated it is. It's weird having that change in a commit (when merged to master) called "Deprecate Electron support", when it's totally unrelated. This also makes it harder when I do release notes, as I have to remember to mention that change manually instead of np including the commit title for me. In addition, it makes it harder to review as I need to sometimes guess whether a change is related to that fix or to the Electron stuff.

Basically the fix is this: /pull/995/files#diff-7bf54112e60a8714e49d3e1e98cf4e2bR32-R36

Looks like it's also: https://github.com/sindresorhus/got/pull/995/files#diff-267e4380ccf0cfebc8b885a948acf226L142-L149 And some other smaller changes.

[sindresorhus]

Regarding the fix for #990, feel free to just commit that directly to master. It doesn't have to be a PR, just a commit with a good title.

[sindresorhus]

Can you add a link to #899? So people can easily know why we are doing this.