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

Wrong flow binary/version used with multi-root vscode workspaces #201

Closed
spudly opened this issue Jan 19, 2018 · 6 comments
Closed

Wrong flow binary/version used with multi-root vscode workspaces #201

spudly opened this issue Jan 19, 2018 · 6 comments

Comments

@spudly
Copy link

spudly commented Jan 19, 2018

When multi-root workspaces are used in vscode, this extension will use the same flow binary for all packages, even if useNPMPackagedFlow is enabled and the workspace folders have different flow versions defined in their package.json files.

This results in a mismatch between the flow errors you see reported in VSCode and the errors that are reported by running flow from the command-line with npx flow. In addition, when you run flow from the command-line, flow catches the version mismatch, kills the old server, and starts a new server. This results in wasted time waiting for flow to initialize over and over.

This happens for two reasons:

  1. The extension doesn't look in each workspace folder for the flow binaries. It currently uses the deprecated workspace.rootPath property instead of workspace.workspaceFolders[].uri, so it only gets the flow binary for the first folder in the workspace. I expect, though I have not experienced this, that if the first folder in the workspace didn't have flow installed, the extension would fail to find the flow binary even if other folders in the workspace had it installed). See: https://github.com/flowtype/flow-for-vscode/blob/dceceae29dc8ccb495b92660fe84746f4f5839ee/lib/pkg/flow-base/lib/FlowHelpers.js#L187
  2. The extension caches the path to the flow binary, but it caches it for the entire workspace and not for each folder in that workspace. There needs to be a cache for each folder. See: https://github.com/flowtype/flow-for-vscode/blob/dceceae29dc8ccb495b92660fe84746f4f5839ee/lib/pkg/flow-base/lib/FlowHelpers.js#L161
@spudly spudly changed the title Wrong flow binary used with multi-root vscode workspaces Wrong flow binary/version used with multi-root vscode workspaces Jan 19, 2018
@rattrayalex-stripe
Copy link

I'm seeing this too. I hope to submit a patch soon.

@rattrayalex-stripe
Copy link

Actually, my plan is to, for each file, look for the nearest package.json which has flow-bin declared as a (dev) dependency, and use that (fallback to the one at workspace root / global) (and then cache the result per directory).

For our use-case, vscode multi-root workspaces look like a pain and this is all we needed them for anyway.

@spudly would that work for you as well?

@spudly
Copy link
Author

spudly commented Jan 30, 2018 via email

@HyperBrain
Copy link

For me it seems that the extension is not multi-root workspace capable at all. Only the very first contained project is used, with some strange results:

If I do not have a flow enabled project as first project, the extension will even fail to locate a local flow installation and tries to resort to a global flow.

I think the task should be extended to make it multi-root compatible and not just fix the local flow lookup. There might be other places that silently fail or just use the wrong data.

@bdrobinson
Copy link

@rattrayalex-stripe did you get anywhere with this? Would be really handy!

@rattrayalex-stripe
Copy link

Sorry, no. I've been hoping the bigger LSP changes would pave the groundwork for this, if not include the fix directly.

Mayank1791989 added a commit to Mayank1791989/flow-for-vscode that referenced this issue Feb 16, 2019
Client (closes flow#302, flow#304, 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
Mayank1791989 added a commit to Mayank1791989/flow-for-vscode that referenced this issue Feb 18, 2019
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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants