tide icon indicating copy to clipboard operation
tide copied to clipboard

Adding Yarn 2 PnP support

Open ramblehead opened this issue 5 years ago • 7 comments

This patch allows tide to jump to files stored in zip packages (in Yarn 2 cache) using Emacs arc-mode.

See #388

(This PR replaces closed PR #392)

ramblehead avatar Jul 28 '20 20:07 ramblehead

I don't know much about what yarn 2 pnp means, so it would be nice if you can add more info about it in the PR description. Specially want to know if it follows any special file name convention etc.

ananthakumaran avatar Jul 29 '20 03:07 ananthakumaran

@ananthakumaran Thank you for the tide mode, and for your reply to this "accidental PR"!

To me tide was (and still is) the first proper environment for JavaScript development (via typescript of-course). And, yarn 2 is the first proper node package management tool - it ticks most of my boxes, including sensible architecture and zero install philosophy on which "PnP" concept is based.

There are many searchable reviews and debates on yarn 2 subject on the Internet. Here I am just assuming that it would be great if at some stage tide could play well with certain yarn 2 peculiarities out-of-the-box :)

To start tide with yarn 2, the following (or similar) emacs config is required:

(setq-local tide-tsserver-executable
            ;; assuming that `project-root', the path to yarn 2 repository, is defined elsewhere
            (concat project-root
                    "/.yarn/sdks/typescript/bin/tsserver"))

a .yarn/sdks bit is described here: https://yarnpkg.com/advanced/editor-sdks

Yarn 2 eliminates recursive node_modules from its repositories. Instead, it is using flat .yarn/cache which looks like the following:

.yarn/cache$ tree
.
├── abbrev-npm-1.1.1-3659247eab-9f9236a3cc.zip
├── abort-controller-npm-3.0.0-2f3a9a2bcb-cc53ad8df9.zip
├── accepts-npm-1.3.7-0dc9de65aa-2686fa30db.zip
├── acorn-jsx-npm-5.2.0-4c0af33483-1247cc4b32.zip
├── acorn-npm-6.4.1-77905520a8-7aa4623c6d.zip
...
├── next-npm-9.4.5-canary.43-21cd24de92-0fbddd70d8.zip
...

Each external node package (not necessarily from npm) is stored in zip archive. Those archives can be uploaded to SCM (e.g. git) together with the repository source code - thus zero install - just clone and build node repository or the whole monorepository. This node packages structure is called PnP in yarn 2 talk.

The following screenshot shows how tide sees files in such zip packages (notice “next” path in dir tree above and on the screenshot in eldoc):

Screenshot from 2020-07-29 13-53-43

If we strip $$virtual/*/0/ part from what tide gets from tsserver, it becomes a legitimate path within .yarn/cache zip archive. If such zip path is visited in emacs, tramp mode tries to open it - at least that is what happens in my environment. Unfortunately, current tramp archives handling is a bit unstable and has issues (such as files in archives are read-only).

The code proposed in this PR does two things:

  1. strips $$virtual part of file paths in yarn 2 zip packages;
  2. and opens code files (e.g. from tide-jump-to-definition) in those zip packages using arc-mode instead of the default tramp handling.

What I do not understand here is how to correctly treat $$virtual part of zip path (similar to your question on “special file name convention etc.”) and if it is safe to just strip it. So far, stripping worked fine with my yarn 2 experimental projects. Probably, need to ask this question to Yarn 2 community...

ramblehead avatar Jul 29 '20 15:07 ramblehead

I found what is the purpose of $virtual in yarn 2: https://yarnpkg.com/advanced/pnpapi#resolvevirtual https://yarnpkg.com/advanced/lexicon#virtual-package

and in yarn 2 code: https://github.com/yarnpkg/berry/blob/5f0439a087b8463d97fb70c96dc6f983a91be512/packages/yarnpkg-fslib/sources/VirtualFS.ts#L14

Basically, the virtual part of the package path is used in yarn 2 to track peerDependencies. To get the real package file path it is ok to strip the virtual part - that is what this PR does.

ramblehead avatar Jul 29 '20 21:07 ramblehead

I too am a fan of both tide and yarn2. Would like to see support in tide. If there's anything I can do to help, test, debug, whatever, LMK.

jkolyer avatar Oct 17 '20 01:10 jkolyer

Does this have a chance of getting merged in? It'd be nice to have Yarn 2 support without having to maintain one's own local fork.

rsimoes avatar Dec 08 '20 08:12 rsimoes

I'm not opposed to adding yarn-support to tide, but for the maintainers to merge this I think we would need at least the following 3 things:

  • all conflicts against git master to be resolved
  • clear instructions on how this is supposed to be used (maybe a purpose-built demo-repo using yarn can be used as a demo?), written for someone who has never before used yarn.
  • clear instructions on how to test the provided changes (again, maybe with a purpose-built demo-repo), written for someone who has never before used yarn.

Basically the current form of this PR puts the burden of all those things onto the maintainers, instead of the contributor, and that's often the number one reason for closed PRs.

So anything which lowers the effort required to "OK" this is probably going to help this get merged.

josteink avatar Dec 08 '20 09:12 josteink

As it turns out, $$virtual/*/0/ part cannot be just striped out from the yarn 2 virtual path: https://github.com/yarnpkg/berry/issues/499#issuecomment-539458981

The algorithm explained by arcanis at the link above is quite simple - we just need to implement it in elisp. Otherwise, I have been using this patch via advice-add since summer in a mid-size multi-package yarn 2 project and quite happy with it:

https://github.com/ramblehead/.emacs.d/blob/c585bd2cb73ced0985b10834779825cbecdb12d2/lisp/config-tide.el#L66

The major limitation of this patch is that once inside a yarn 2 package archive, tide does not work there and if one needs to navigate/refactor/etc. within packages she would need to unplug them first. I do not know, even conceptually, how to make tide (or lsp) work from within archives...

@josteink thank you for the points outlining contribution requirements - I will try to find a bit of time to follow them...

ramblehead avatar Dec 09 '20 18:12 ramblehead