ananthakumaran / tide

Tide - TypeScript Interactive Development Environment for Emacs
GNU General Public License v3.0
1.45k stars 108 forks source link

Support for Yarn PnP #388

Open gamb opened 4 years ago

gamb commented 4 years ago

Just creating this as a stub. It'd be really neat if tide could support Yarn PnP, where files are zipped inside .yarn/cache.

Quick and dirty solution that works (but relies on you already having the zipped buffer open):

(defun tide-get-file-buffer (file &optional new-file)
  "Returns a buffer associated with a file. This will return the
current buffer if it matches `file'. This way we can support
temporary and indirect buffers."
  (cond
   ((equal file (tide-buffer-file-name)) (current-buffer))
   ((file-exists-p file) (find-file-noselect file))
   (new-file (let ((buffer (create-file-buffer file)))
               (with-current-buffer buffer
                 (set-visited-file-name file)
                 (basic-save-buffer)
                 (display-buffer buffer t))
               buffer))
+   ((string-match "\\$\\$virtual.*cache/" file)
+   (find-file-noselect (replace-regexp-in-string "\\$\\$virtual.*cache/" "cache/" (replace-regexp-in-string "\\.zip/" ".zip:" file))))
   (t (error "Invalid file %S" file))))

It is possible to yarn unplug dependencies to unzip them, but it seems to me that we should be able to leverage Emacs ability open buffers inside zips.

gamb commented 4 years ago

Example response with PnP:

1 -> (tide-response-success-p (:seq 0 :type "response" :command "definition" :request_seq "589" :success t :body ((:file "/home/gamble/path/to/project/.yarn/$$virtual/recoil-virtual-ad5204a645/0/cache/recoil-npm-0.0.10-9811d22459-2.zip/node_modules/recoil/dist/index.d.ts" :start (:line 117 :offset 17) :end (:line 117 :offset 31) :contextStart (:line 117 :offset 1) :contextEnd (:line 117 :offset 89)))))

The response path is transformed from:

.yarn/$$virtual/recoil-virtual-ad5204a645/0/cache/recoil-npm-0.0.10-9811d22459-2.zip/node_modules/recoil/dist/index.d.ts

To an Emacs-friendly path (on disk):

.yarn/cache/recoil-npm-0.0.10-9811d22459-2.zip:node_modules/recoil/dist/index.d.ts
ramblehead commented 4 years ago

@gamb I have done a similar fix just yesterday with advice-add() in tide-jump-to-filespan(). tide-get-file-buffer() seems to be a better fix point.

I hesitated to publish Subj. issue here as there were still a few questions which I could not answer:

@gamb what is your experience with editing files in archives (i.e. not just a singe compressed file) from Emacs without unpacking them? Do you use tramp or tar/archive modes?

May be we should have an option to "auto-unplug" yarn 2 zip package from tide-get-file-buffer() instead of opening from zip?

ramblehead commented 4 years ago

Here is a "naive" implementation of tide-get-file-buffer() which can open files in zip archives using archive mode, stripping /$$virtual/... part, avoiding tramp and auto-opening "zip buffer":

(defun tide-get-file-buffer:override (file &optional new-file)
  "Returns a buffer associated with a file. This will return the
current buffer if it matches `file'. This way we can support
temporary and indirect buffers."
  (cond
   ((equal file (tide-buffer-file-name)) (current-buffer))
   ((string-match-p ".*\\.zip/.*" file)
    (let* ((full-path
            (replace-regexp-in-string "\\$\\$virtual.*cache/" "cache/" file))
           arc-path file-path-in-arc arc-buf)
      (save-match-data
        (string-match "\\(.*\\.zip\\)/\\(.*\\)" full-path)
        (setq arc-path (match-string 1 full-path))
        (setq file-path-in-arc (match-string 2 full-path)))
      (setq arc-buf (find-file-noselect arc-path))
      (with-current-buffer arc-buf
        (beginning-of-buffer)
        (search-forward file-path-in-arc)
        (archive-extract))))
   ((file-exists-p file) (find-file-noselect file))
   (new-file (let ((buffer (create-file-buffer file)))
               (with-current-buffer buffer
                 (set-visited-file-name file)
                 (basic-save-buffer)
                 (display-buffer buffer t))
               buffer))
   (t (error "Invalid file %S" file))))

(advice-add 'tide-get-file-buffer :override
            #'tide-get-file-buffer:override)

I tested it briefly and it seems to work well in my stings...

gamb commented 4 years ago

Nice one @ramblehead :). Sorry I haven't looked at this again yet. Really quick thoughts...

  1. Need to check Yarn implementation to see if $$virtual path covers all the cases.
  2. ((string-match-p ".*\\.zip/.*" file) looks good - could check it exists too (in the cond)? Generalized support for zips feels like a better aim than specific PnP support
ramblehead commented 4 years ago

Good idea on generalized support for zips! I would extend it further on all archives supported by arc-mode. Also, there is a possibility of having a directory-name.zip which would not work. Here is another refactoring cycle to use archive signatures instead of file naming patterns and checking for non-files:

(require 'arc-mode)

(defun tide--get-arc-path-pair (full-path)
  ;; not sure if "|\\\\" is need - do not have M$ Windows to check...
  (let ((path-components (split-string full-path "/\\|\\\\"))
        (arc-path "")
        (file-path-in-arc "")
        arc-found)
    ;; Distinguishing absolute and relative paths - i.e. trailing "/".
    (unless (string-empty-p (car path-components))
      (setq arc-path (car path-components)))
    (setq path-components (cdr path-components))
    (seq-do
     (lambda (component)
       (if arc-found
           (setq file-path-in-arc (concat file-path-in-arc "/" component))
         (setq arc-path (concat arc-path "/" component))
         (when (and (file-regular-p arc-path)
                    (with-temp-buffer
                      ;; 300000 is a magic number - it should
                      ;; be more than enough to recognise any achieve
                      ;; type header.
                      (insert-file-contents arc-path nil 0 300000)
                      (ignore-errors (archive-find-type))))
           (setq arc-found t))))
     path-components)
    (and arc-found
         (not (string-empty-p arc-path))
         (not (string-empty-p file-path-in-arc))
         (cons arc-path (substring file-path-in-arc 1)))))

(defun tide-get-file-buffer:override (file &optional new-file)
  "Returns a buffer associated with a file. This will return the
current buffer if it matches `file'. This way we can support
temporary and indirect buffers."
  (let (arc-path-pair)
    (cond
     ((equal file (tide-buffer-file-name)) (current-buffer))
     ((setq arc-path-pair
            (tide--get-arc-path-pair
             (replace-regexp-in-string "\\$\\$virtual.*cache/" "cache/" file)))
      (let ((arc-path (car arc-path-pair))
            (file-path-in-arc (cdr arc-path-pair))
            arc-buf)
        (setq arc-buf (find-file-noselect arc-path))
        (with-current-buffer arc-buf
          (goto-char (point-min))
          ;; This should fail in nested archives.
          (search-forward file-path-in-arc)
          (archive-extract))))
     ((file-exists-p file) (find-file-noselect file))
     (new-file (let ((buffer (create-file-buffer file)))
                 (with-current-buffer buffer
                   (set-visited-file-name file)
                   (basic-save-buffer)
                   (display-buffer buffer t))
                 buffer))
     (t (error "Invalid file %S" file)))))

(advice-add 'tide-get-file-buffer :override
            #'tide-get-file-buffer:override)
ramblehead commented 4 years ago

Here is a patch for Yarn 2 paths shown in eldoc

(defun tide-eldoc-maybe-show:override (text)
  (with-demoted-errors "eldoc error: %s"
    (and (or (tide-eldoc-display-message-p)
             ;; Erase the last message if we won't display a new one.
             (when eldoc-last-message
               (eldoc-message nil)
               nil))
         (eldoc-message (replace-regexp-in-string
                         "\\$\\$virtual.*\\(cache/\\)" "\\1" text)))))

(advice-add 'tide-eldoc-maybe-show :override
            #'tide-eldoc-maybe-show:override)

Both patches work well on my system. Would be great if more people could test it. Anyone interested in using tide with yarn 2 PnP packages?

Edit: changed to tide-eldoc-display-message-p

josteink commented 4 years ago

Shouldn't you be using the tide-specific eldoc-function you just sent a PR for? 😅

Edit: Also no need to advice a function we already own and control. Better to just update the function itself.

ramblehead commented 4 years ago

I thought to keep two issues separate. Also, not sure if it is the best point to filter edoc messages, but can't find any better.

Advice is for folks waning to test this patch quickly by copy'n'paste and evaluate in *scratch* buffer.

I have been using this patch for the whole evening now :P Can now jump around in to Yarn 2 PnP zip files and go back. Only need to disable tide in buffers where (bound-and-true-p archive-subfile-mode) is t. Another PR? :)

josteink commented 4 years ago

It's merged now, so at this point it's required to use 👍

I really don't know enough about this scenario to do a solid review, but if you submit another PR for this, someone else might step up.

Give it a try?

gamb commented 3 years ago

@asteroidcow Noticed your commit in https://github.com/ananthakumaran/tide/compare/master...asteroidcow:pnp I don't have time to try it out just now, but will later on. Do you plan to open a PR for this?

ramblehead commented 3 years ago

@gamb the @asteroidcow code seems to be coming from my push request #394 I have done some more refactoring since then and extracted "yarn 2 - related tide augmenting" to a separate package: https://github.com/ramblehead/.emacs.d/blob/384f20664468650a9c99b41c9aba1df73864a19b/lisp/tide-yarn2.el so tide can be updated from upstream preserving yarn 2 changes.

I would suggest to use the code from the above tide-yarn2.el package as it handles yarn2 virtual paths correctly, see:

Separate package/advice-add is not perfect - I would rather see those changes in upstream, but as @josteink rightfully advised, further work is required to provide usage instructions, demo repositories and tests (any help on that is welcome).

I am using tide-yarn2.el package myself since March and have no issues with it. My only "wish" is that tide could work inside zip archives with no unplug - not sure how it can be done though.

asteroidcow commented 3 years ago

Yup, @gamb I merged @ramblehead 's code in my local fork. It's not my work. I just needed a repo to point straight.el to for installation.

As an fyi, the monorepos I work with use Yarn2 PnP + workspaces, and I did have to do some more hacky and completely non-portable things to the patch to make it work with workspaces.

Overall, I can jump to definitons and get type information for my code and library code. What I did notice is that I couldn't jump to a definition from one library code to another library code.

Maybe the additions that @ramblehead mentioned above might mean I don't need to do this hacky stuff anymore -- will test it out this week.

Here are the additional things I had to add to my init.el (note: I barely know any elisp)

(defun tide-yarn2-setup ()
  "Configure executables for yarn pnp."
  (interactive)
  (setq
   yarn-root (substring
             (shell-command-to-string "yarn config get virtualFolder")
             0
             (- (length "/$$virtual")))
   tide-node-executable "/home/johndoe/.local/bin/yarnode"
   tide-tscompiler-executable (concat yarn-root "sdks/typescript/bin/tsc")
   tide-tsserver-executable (concat yarn-root "sdks/typescript/bin/tsserver")
   tide-tsserver-process-environment '("TSS_LOG=-level verbose -file /tmp/tss.log")
   )
  )

where yarnode is a shell script which runs:

yarn node $@
ramblehead commented 2 years ago

Further update on yarn 2 and yarn 3 pnp developments! yarn 3 introduces a slightly different virtual paths schema that breaks all previous code - including my tide-yarn2.el and PR.

I now have a new emacs package that adds yarn 2 and 3 pnp support to both tide and lsp: https://github.com/ramblehead/.emacs.d/blob/master/lisp/yarn-pnp.el

yarn-pnp can be loaded after tide or lsp as the following:

(require 'yarn-pnp)

(yarn-pnp-tide-enable) ; for tide
;; or
(yarn-pnp-lsp-enable) ; for lsp

yarn-pnp package is not a minor mode - it is a bunch of advice-add hacks. Those hacks solve two yarn pnp problems - resolving yarn virtual paths and opening zip package files with arc-mode instead of default tramp (that I could never make working satisfactory with archives).

I am not sure if solving yarn-pnp -- specific problems should be done in tide or lsp code. To me it seems a bit out-of-scope for those projects. Should we instead add hooks into tide, lsp and eglot that can be used by other packages such as yarn-pnp to:

If such hooks can be added, then yarn-pnp can be refactored into a stable yarn pnp solution instead of relying on advice-add that may break at any time if advised upstream functions change.

ramblehead commented 2 years ago

My first attempt to create an example Yarn 3 repository that uses yarn-pnp.el package: https://gitlab.com/ogorod/next-app-rh