Support browsers from before 2020

[L3P3]

As reported in #2825, #2826 and #3051, almost everything works in older browsers. This setting here prevented me from updating and I think it is an obvious enhancement to not artificially restrict to browsers from 2020+. There should not be any noticable downsides of this change since es6 and es2020 are pretty similar with only minor differences.

[jsjoeio]

@code-asher I can't think of any downsides to this, can you? 🤔

[jawnsy]

I guess upstream vscode uses es2020 as the target, so one risk is that divergence means that the result may not be as well-tested. I don't know enough to have an accurate assessment of the risk though.

[jsjoeio]

I guess upstream vscode uses es2020 as the target, so one risk is that divergence means that the result may not be as well-tested. I don't know enough to have an accurate assessment of the risk though.

Great point! I'm guessing we decided to use es2020 after they upgraded. Looks like they dropped support for legacy edge and IE when they made that upgrade.

[code-asher]

We can change our target but code-server will still not work since VS Code has their own target. I am not sure we even have any of our own client-side JavaScript anymore in 4.0.0.

[jsjoeio]

We can change our target but code-server will still not work since VS
Code has their own target.

Ah right! This would have to be in VS Code itself (or in our fork). But if we change in our fork, that would diverge from upstream and could cause other issues.

I am not sure we even have any of our own
client-side JavaScript anymore in 4.0.0.

Wow, you're right. I didn't even realize that.

[L3P3]

Between es6 and es2020, there are no interoperability risks. In this case, they only differ in syntactic sugar like ?? and ?. operators. Not even polyfills required to ensure compatiblity between two modules.
Isn't this tsconfig also respected in submodules? I think it should be at least tested if that works. Or are you sure that there will still be es2020 code after this proposed change?

[L3P3]

I am not sure we even have any of our own client-side JavaScript anymore in 4.0.0.

What about the forked repository? Isn't that deemed "our own"?

But if we change in our fork, that would diverge from upstream and could cause other issues.

Yeah, we diverge in one global setting but TypeScript guarantees that older targets are still working properly and being compatible to newer code... I think code-server has a selling point in being more compatible than upstream. Microsoft does not care about a few individuals that diverge from mainstream for some reason. We have the chance to continue to not go that route with probably very low effort.

[code-asher]

VS Code is compiled separately using their build process so our tsconfig has no effect on it. But if we patch it there instead and everything works that seems like a win to me. We could also leave this change here as well in case we ever add client-side JavaScript again. Our tests are reporting the following with this change: ``` test/unit/node/cli.test.ts:91:11 - error TS2550: Property 'flat' does not exist on type '(string | string[])[]'. Do you need to change your target library? Try changing the 'lib' compiler option to 'es2019' or later ``` So maybe we also need to add `lib`.

[L3P3]

So maybe we also need to add lib.

Yes, sure. Those are polyfills so that beside syntax change, also new methods work in older browsers. 😉

[jsjoeio]

But if we patch it there instead and everything works that seems like a
win to me.

Good point! I agree. @code-asher should we ask @L3P3 to submit a PR to our fork here?

We could also leave this change here as well in case we ever add
client-side JavaScript again.

Agreed. As long as the build passes, I think we're okay to merge.

[jsjoeio]

Even though we don't have any client-side JS in code-server at the moment, we're okay merging this in, in case we do want to support older browser in the future.

Thanks @L3P3 for adding this 👍

[jsjoeio]

Docs preview

P.S. you can ignore this. It's a known issue that I need to fix.

[L3P3]

Sounds great. Done.

Lets hope it compiles successfully. 🙈

[L3P3]

BTW if anyone wonders why they targeted es2020 in the first place: They use electron which is pretty much latest chrome. Web development and electron development has some differences. 😉

[jsjoeio]

FAIL test/unit/node/routes/login.test.ts
  ● Test suite failed to run

    test/unit/node/routes/login.test.ts:81:9 - error TS2322: Type 'URLSearchParams' is not assignable to type 'BodyInit | undefined'.
      Type 'URLSearchParams' is missing the following properties from type 'URLSearchParams': entries, keys, values, [Symbol.iterator]

    81         body: params,
               ~~~~

Hmm... looks like one of the tests are failing. You might need to look into fixing login.test.ts

[jsjoeio]

I still see the same error in the test here. We'll push this for the next release.

[L3P3]

Sorry, I thought that dom includes dom.iterable but it appears to not be the case. Just another try and hopefully no error. 🙈

[codecov]

Codecov Report

Merging #4579 (7a441d1) into main (1b796d1) will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #4579      +/-   ##
==========================================
- Coverage   66.14%   66.10%   -0.04%     
==========================================
  Files          30       30              
  Lines        1654     1658       +4     
  Branches      331      337       +6     
==========================================
+ Hits         1094     1096       +2     
- Misses        475      477       +2     
  Partials       85       85              

Impacted Files	Coverage Δ
src/node/routes/pathProxy.ts	67.85% <0.00%> (-1.38%)	⬇️
src/node/cli.ts	83.52% <0.00%> (-0.26%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b796d1...7a441d1. Read the comment docs.

[jsjoeio]

@code-asher tested this yesterday and I think it should be okay to merge after 4.0.0 is out!