fix(lib/vscode): remove symlink in npm package

[jsjoeio]

This PR removes the symlink to node_modules.asar when packaging up for npm because as of recently, npm no longer allows symbolic links in packages (we also don't need it).

Why are we doing this?

On Friday, August 6th, we released 3.11.1.

The npm workflow did not work as expected leading to 3.11.1 not being published on npm, Homebrew, or AUR. We did some investigation and came to the conclusion something changed on the npm side with regard to symlinks in packages.

We spoke to someone today, August 9th from GitHub who confirmed our theory.

Hi Joe! Saw your tweet about symlinks in npm packages. We did make some changes here recently. Can you drop me a line so that we can work through how to support you?

A few of the maintainers chatted and we came to the conclusion that we can remove this symlink.

We rm -f this symlink in the post-install and recreate it (to make sure it's correct) so the packaged link will never be used.

—@code-asher

This PR removes that symlink.

Why did we have this before?

If you take a look at lib.sh, you'll see previous maintainers left a comment explaining why we do this.

VS Code bundles some modules into an asar which is an archive format that
works like tar. It then seems to get unpacked into node_modules.asar.

I don't know why they do this but all the dependencies they bundle already
exist in node_modules so just symlink it. We have to do this since not only VS
Code itself but also extensions will look specifically in this directory for
files (like the ripgrep binary or the oniguruma wasm).

You sure this won't break anything?

90% sure it shouldn't break anything because we actually remoe the symlink in the post-install and recreate it anyway so the one included in the npm package is never actually used.

There may be an edge case or two that we didn't anticipate but I (@jsjoeio) removed it, published to npm, tested locally and did not find any major issues.

How I tested locally

1. yarn
2. yarn build
3. yarn build:vscode
4. yarn release
5. check for symlinks - cd release && find . -type l -ls

Then I tested the individual release:

1. cd release
2. yarn
3. node .

Screenshot and video

Screen.Recording.2021-08-09.at.12.18.56.PM.mov

Fixes N/A

Related:

• the GitHub Action log where it broke
• our investigation notes
• tweet asking for help

Other notes

@jawnsy showed me this trick to find symlinked files in a directory.

find . -type l -ls

Helpful for debuggging this kind of thing.

[codecov]

Codecov Report

Merging #3935 (9d83659) into main (741b834) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #3935   +/-   ##
=======================================
  Coverage   63.51%   63.51%           
=======================================
  Files          36       36           
  Lines        1872     1872           
  Branches      379      379           
=======================================
  Hits         1189     1189           
  Misses        580      580           
  Partials      103      103           

---

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 741b834...9d83659. Read the comment docs.

[jawnsy]

Love the writeup! I trust you :)

[greyscaled]

Seems fine to me, but I don't know what I don't know w.r.t to the fork solution and testing this.

the ripgrep ext worked, but I have no way to know if some other ext breaks :(

[jsjoeio]

I don't know w.r.t to the fork solution and testing this.

I think @TeffenEllis mentioned once the fork solution is implemented, we won't even have to worry about this symlink issue cause it's non-existent there.

the ripgrep ext worked, but I have no way to know if some other ext breaks :(

Yeah, worked for me too! I guess we'll have to release and see what happens? If something does break, we can figure it out though. And eventually, we'll be able to write more tests for specific extensions (once #3621 gets merged)