Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support local flow-bin #12

Open
wpcarro opened this issue Feb 16, 2018 · 5 comments
Open

Support local flow-bin #12

wpcarro opened this issue Feb 16, 2018 · 5 comments

Comments

@wpcarro
Copy link

wpcarro commented Feb 16, 2018

Any plans to support the local flow-bin executable via something like npx flow ...? That would be nice, so that project-specific flow-bin versions could be used instead of depending on a globally installed version. Other editor tools in the npm ecosystem support this if we need to draw from a precedent.

@aaronjensen
Copy link
Owner

I use this:

(defun flow/set-flow-executable ()
  (interactive)
  (let* ((os (pcase system-type
               ('darwin "osx")
               ('gnu/linux "linux64")
               (_ nil)))
         (root (locate-dominating-file  buffer-file-name  "node_modules/flow-bin"))
         (executable (car (file-expand-wildcards
                           (concat root "node_modules/flow-bin/*" os "*/flow")))))
    (setq-local flow-minor-default-binary executable)
    (setq-local company-flow-executable executable)
    (setq-local flycheck-javascript-flow-executable executable)))

(add-hook 'rjsx-mode-hook #'flow/set-flow-executable t)

I'm not sure about supporting this in the library out of the box, but maybe that'd be ok.

@wpcarro
Copy link
Author

wpcarro commented Feb 16, 2018

Thanks for sharing this snippet! I'll certainly be using this myself. From what I've seen, I know many people prefer using local bins whenever possible. Using npx is a great way to access the node_modules/.bin/flow call, which you may be able to use to simplify some of the finding logic in your function.

From a design philosophy, why not support a variable for activating the preference?

E.g. -

(setq company-flow-prefer-local-bin t)

@aaronjensen
Copy link
Owner

Personally, I wouldn't use npx for this as it increases startup time:

$ time npx flow version
Flow, a static type checker for JavaScript, version 0.65.0
npx flow version  0.14s user 0.11s system 95% cpu 0.263 total

$ time ./node_modules/.bin/flow version
Flow, a static type checker for JavaScript, version 0.65.0
./node_modules/.bin/flow version  0.10s user 0.07s system 96% cpu 0.172 total

$ time ./node_modules/flow-bin/flow-osx-v0.65.0/flow version
Flow, a static type checker for JavaScript, version 0.65.0
./node_modules/flow-bin/flow-osx-v0.65.0/flow version  0.01s user 0.01s system 84% cpu 0.023 total

Using the bin for flow directly is slower as well. That's why I path to the actual binary.

For this library, building that in may make sense since the logic is somewhat complicated and it makes a big difference in the performance/usability of company-flow.

@aaronjensen
Copy link
Owner

I updated the readme w/ the snippet. Hopefully that helps some people, but I'll still consider building this in.

@wpcarro
Copy link
Author

wpcarro commented Feb 16, 2018

Thanks for the explanation. I think I share your concerns about the time that npx pads in. I will definitely make use of that snippet for now and I'll stay tuned for any package updates!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants