Skip to content
This repository has been archived by the owner on Dec 19, 2024. It is now read-only.

[LSP] Add multi config & vscode multi-root workspace support #315

Merged
merged 16 commits into from
Feb 27, 2019

Conversation

Mayank1791989
Copy link
Contributor

Client

New implementation will handle following cases

  • Single workspace with multiple flowconfig (multi-config)
# some single workspace project

vscode-root
  └──  project-A                [create flow client for this dir]
        └── src                 (contains js files)
        └── .flowconfig
  └── some-dirA
        └── project-c           [no .flowconfig so no client]
              └── src           (contains js files)
              └── test.js
  └──  some-dir
        └── project-B           [create flow client for this dir]
              └── src           (contains js files)
              └── .flowconfig
              └── test.js
  • Multiple workspace with single|multiple flowconfig (vscode multi-root workspace)
# some multiple workspace project

vscode-workspace-folder-a (all clients below will use this worskspaceFolder settings)
  └──  project-aa               [create flow client for this dir]
        └── src                 (contains js files)
        └── .flowconfig
  └── some-dirA
        └── project-ab          [no .flowconfig so no client]
              └── src           (contains js files)
              └── test.js
  └──  some-dir
        └── project-ac           [create flow client for this dir]
              └── src           (contains js files)
              └── .flowconfig
              └── test.js

vscode-workspace-folder-b (all clients below will use this worskspaceFolder settings)
  └──  project-ba               [create flow client for this dir]
        └── src                 (contains js files)
        └── .flowconfig
  └── some-x-dir
        └── project-b           [no .flowconfig so no client]
              └── src           (contains js files)
              └── test.js
  └──  some-y-dir
        └── project-c           [create flow client for this dir]
              └── src           (contains js files)
              └── .flowconfig
              └── test.js

(fixes #302, #308, #253, #228, #201, #195, #189, #184, #165, #81)

Resolve Flow-Bin

Error handling (fixes #200)

  • Improve error handling.
  • Show action in errors so that user can recover from error without restarting vscode

Commands

  • Add commands
    • Toggle coverage
    • Show client status
    • Restart Client
    • Log Client Debug Info
    • Show output channel

Status Bar Widget

Include more info in status widget to help user know what this plugin is using

  • Include flow version
  • Add flow info section in widget tooltip which includes
    • path to .flowconfig
    • flow version
    • path to "flow" binary

Vscode settings

  • flow.coverageSeverity to control type coverage diagnostic severity
  • flow.useBundledFlow enable|disable use of flow bundled with this plugin
  • flow.lazyMode: to add flow lazyMode support
  • flow.trace.server log communication between vscode and flow lsp
  • flow.logLevel control log level of plugin logs

Docs

  • Update readme

Build | Tools

  • Change babel target node to 8 (previously we are compiling for node 6)
  • Update flow (bundled) to 0.93
  • Update vscode-languageclient to 5.2
  • Update vscode requirement to match vscode-languageclient requirement.
  • Add linting (eslint + prettier)

Other Changes

  • move non lsp code into directory

vscode (>=1.30.0) already using node 8. So no point of transforming code
using node 6.

other changes
- remove regenerator-runtime (node 8 supports async/await)
Client (closes flow#302, flow#308, flow#253, flow#228, flow#201, flow#195, flow#189, flow#184, flow#165, flow#81)
- add multi flowconfig support
- add vscode multi-root workspace support
- client is now lazily created if flowconfig found
- fix document selector.

New implementation will handle following cases
- Single workspace with multiple flowconfig
- Multiple workspace with multiple flowconfig

Get Flow-Bin
- improve|fix logic to find flow-bin (fixes flow#282, flow#240, flow#209)
- fix flow bundled with plugin not working
  issue: we are using .bin/flow but vscode never includes .bin in plugin package
- add logs while finding flow-bin to debug issues
- plugin now doesn't check for node (closes flow#290)
- pathToFlow: value is normalized and .cmd is added so same value can
be used for linux, osx & win

Error handling (closes flow#200)
- [feat] improve error handling.
- [feat] Show action in errors so that user can recover from error
  without restarting vscode

Commands
- [feat] Add commands
  - Toggle coverage
  - Show client status
  - Restart Client
  - Log Client Debug Info
  - Show output channel

Status Widget
Include more info in status widget to help user know what this plugin is using
- [feat] include flow version
- [feat] add flow info section in widget tooltip which includes
  - path to .flowconfig
  - flow version
  - path to "flow" binary

Settings
- flow.coverageSeverity to control type coverage diagnostic severity
- flow.useBundledFlow control use of flow bundled with this plugin
- flow.trace.server log communication between vscode and flow lsp
- flow.logLevel control log level of plugin logs
@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@thymikee
Copy link
Contributor

Whoa this is so cool! I need to find some time to take a look at it. @orta have you seen this?

@jfbrazeau
Copy link

Is it possible to know if this PR will be merged in the coming days ? Not being able to handle multi root workspaces in VSCode is a strong limitation for somebody (like me) who plans to use Flow. As far as I can see, it is even probably a strong limitation for using flow itself as most people use VSCode today.

Il like the "flow" approach about static type checking. I would really like to give it a chance, but I must admit that not being able to use it in VSCode with a multi root project could be a justification to switch to Typescript (at least it doesn't help me to argue against colleagues at work).

That's why I would really appreciate if flow-for-vscode team could give a feedback about this PR. Thank you in advance.

@orta
Copy link
Contributor

orta commented Feb 27, 2019

Yeah, I missed this - dang. That's a lot of work!

Ok, looking now

@orta
Copy link
Contributor

orta commented Feb 27, 2019

Alright, so nothing stands out in the code - and I've given it 10m with the dev build inside the Jest codebase just clicking around and nothing seems to be broken. So I'm game to ship this, nice work @Mayank1791989

@orta orta merged commit 89f7115 into flow:master Feb 27, 2019
@orta
Copy link
Contributor

orta commented Feb 27, 2019

Cool, so this is now shipped as 1.1.0 and should be appearing inside vscode pretty soon for folks

@jfbrazeau
Copy link

Great news ! Thank you very much !

@kuus
Copy link

kuus commented Feb 28, 2019

thanks @Mayank1791989!
@orta do you know if this includes support for yarn workspaces?
For instance I have this folder/project structure

/project
/project/.flowconfig
/project/node_modules
/project/packages
/project/packages/core
/project/packages/core/.flowconfig
/project/packages/www
/project/packages/www/.flowconfig

my project/package.json has the following

  "workspaces": {
    "nohoist": [
      "gatsby*"
    ],
    "packages": [
      "packages/*"
    ]
  }

so when I run yarn install from the two packages the modules are actually put in /project/node_modules
instead of in each /package/package_name folder.

In VScode I still get a bunch of errors such Cannot resolve module '@material-ui/core/styles'. from /project/packages/core/MyComponent.js

I am not sure how to setup the .flowconfig files in the root and in each package in order to make flow resolve the imports, I've tried solutions such using:

[options]
module.system.node.resolve_dirname='<PROJECT_ROOT>../../node_modules'
# or
module.system.node.resolve_dirname=../../node_modules
# or
module.system.node.resolve_dirname=./node_modules

and many others but nothing seem to work...

To recap, this is the three .flowconfig files I am trying to figure out:

/project/.flowconfig:

[options]

/project/packages/core/.flowconfig:

[options]

/project/packages/www/.flowconfig:

[options]

hope you can help me understand this, thanks in advance!

@Mayank1791989 Mayank1791989 deleted the feat/multi-config branch March 16, 2019 06:50
@Mayank1791989
Copy link
Contributor Author

@kuus The issue you are facing is the limitation of flow, there is nothing we can do in this plugin.

Have you tried flow-mono-cli It basically creates symlinks of node_modules which are hoisted to top level folder (and as flow supports symlinks it will fix your missing modules errors).

@Mayank1791989
Copy link
Contributor Author

@thymikee and @orta thanks for reviewing and merging the PR.

@fabiomcosta
Copy link

Is there a case where using lazyMode is not recommended?
I guess my question is: shouldn't lazyMode be set to ide by default?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.