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

Add shell: true option for modern Node/VSCode #464

Closed
wants to merge 1 commit into from

Conversation

edemaine
Copy link

Fixes #458 which is necessary for this plugin to work on modern VSCode on Windows.

Personally I still have issues using this plugin in my setup, because of the use of bin-version which calls execa which calls cross-spawn, which doesn't seem to support Flow via yarn on Windows (which creates .js files but no .cmd/.bat` file) — not sure exactly why.

@SamChou19815
Copy link
Member

@edemaine Please be aware that this repo is not actively monitored by the Flow team, and in the last week I have copied the source in the flow repo (facebook/flow@398144a) and will maintain it from there on ward. Therefore, we are likely to archive this repo soon. Do you mind to redo this change in the main flow repo and provide a test plan?

facebook-github-bot pushed a commit to facebook/flow that referenced this pull request Dec 16, 2024
Summary:
As reported automatically in flow/flow-for-vscode#458, this plugin needs to engage the shell to launch the LSP on modern VSCode on Windows, because the script is generally installed as `flow-bin/cli.cmd` and only the shell can execute those.

(port of flow/flow-for-vscode#464)

Personally I still have issues using this plugin in my setup, because of the use of [`bin-version`](https://www.npmjs.com/package/bin-version) which calls [`execa`](https://github.com/sindresorhus/execa) which calls [`cross-spawn`](https://github.com/moxystudio/node-cross-spawn), which doesn't seem to support Flow via `yarn` on Windows (which creates `.js` files but no `.cmd`/.bat` file) — not sure exactly why. In the future, I'd suggest moving away from this rather heavy chain of dependencies. Let me know if you'd like a PR to that effect.

Pull Request resolved: #9242

Reviewed By: SamChou19815

Differential Revision: D67295692

fbshipit-source-id: 68e7f68bb6e2593f3579e00a7bbf402b4be0f014
@SamChou19815
Copy link
Member

Closing since it has landed in the facebook/flow repo already

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

Successfully merging this pull request may close these issues.

Notification of breaking api change with v1.92 release of VS Code
3 participants