Flow code actions blocking save with prettier fixups

[jvilk-stripe]

See the issue I opened here on VS Code:
microsoft/vscode#89745

I believe that, with ESLint now using code actions to implement ESLint fixups with the following option:

"editor.codeActionsOnSave": {
  "source.fixAll.eslint": true
}

(I think this means automatically apply all fixups for errors with source eslint...)

...that VS Code waits for Flow to respond to a code action request before applying ESLint and prettier fixes.

I say "I believe that" because individual extension logs heavily suggest that this is happening. When the slowdown occurs:

• VS Code sends a codeAction request to Flow's language server with eslint diagnostics in the context.
• Flow responds after over a second.
• VS Code does not seem to ask ESLint for fixups until Flow responds, and ESLint only takes ~250 ms.
• Prettier only takes ~50ms.

Also, disabling Flow fixes the issue, which is further evidence that this is happening. With that said, I have not found any VS Code logs that contain information concerning what it is waiting on when the user hits 'save', so I have not explicitly confirmed that my mental model of the issue is correct.

Since the Flow LSP can get backed up typechecking changes and takes 1-2 seconds to respond on large projects, users experience a noticeable delay between hitting save and seeing the final, Prettier-formatted file.

I don't yet know what VS Code's response to my issue will be, or when/if it will be addressed. One workaround that you could provide for Stripe that would be nice:

• A way to disable Flow code actions, e.g. make the LSP server advertise that it does not support code actions to VS Code. We do not make use of them internally.
• Make Flow quick-reject code action requests with a context that only contains non-Flow diagnostics without blocking on any active typechecking operations. There's a source field on Diagnostics that you can use for this purpose.

Regardless of your thoughts on the matter, I thought that this project might want to be made aware of this issue.

[jvilk-stripe]

If you're overburdened but amenable to a pull request, I can put together something extension-side that would accomplish either workaround.

If this should go into Flow's language server itself... I'm not terribly familiar with OCaml, but I could try to muddle my way through adding an option to Flow. The downside of fixing the issue in Flow is that we would have to upgrade to whichever version of Flow ships with the new option.

[jvilk-stripe]

The VS Code folks says this is something the flow extension needs to fix.

microsoft/vscode#89745 (comment)

From your description, this sounds like the bug is on the flow extension side. The flow extension should either:

• Register its code action provider with the correct CodeActionProvider.metadata so the VS Code never invokes it in the first place
• If its code action provider is invoked, quickly check if its actions are not being requested. If they are not, return before doing anything expensive.

[jvilk-stripe]

facebook/flow#8277 describes the solution. You may close this once you've acknowledged this bug, as it'll impact ~most VS Code users of Flow.

[Mayank1791989]

@jvilk-stripe Are you using flow codeActions (experimental.lsp.code_actions: true)?
Note: there was a bug in flow v0.107-0.109 where flow is always returning true for serverCapabilities.codeActionProvider. If you are not using flow codeActions, then upgrading flow to version >=0.110.0 should fix your issue.

[jvilk-stripe]

@Mayank1791989 Thanks for the response. We are already working on upgrading flow to at least v0.111 internally so we can do just that.

A true fix, though, is for Flow's language server to only advertise the code actions it supports during initialization. If it doesn't support the "source.fixAll.eslint" fixup, VS Code won't wait for it on save. It should be a trivial fix for the server.

[jvilk-stripe]

You can close this issue if you're now properly aware of the problem. Since ESLint + Flow seems like it would be a common combination in the VS Code Flow community, it seemed prudent to let the VS Code maintainer(s) of the Flow extension know about this unexpected performance problem so that they could advocate for their users and push for a fix on the language server side of development. :)

[Mayank1791989]

@jvilk-stripe Let's keep this issue open for now. I will put a conditional fix (based on flow version) in the plugin to prevent the issue for developers using flow version v0.107.0-v0.109.0 or >=v0.110.0 with experimental.lsp.code_actions: true (I don't think lot of developers are using experimental.lsp.code_actions, as it's not documented anywhere in flow and also it requires type-first mode to properly work). Anyways the fix is required even if flow releases new version (for developers using older version of flow) so let's add it in plugin also.

[jvilk-stripe]

@Mayank1791989 that would be awesome. There's a blocker internally to rolling out Flow v0.111 everywhere within Stripe, so that would let us re-enable ESLint auto-fixups without waiting for the upgrade. :D

[jvilk-stripe]

btw, the fix should block Flow's language server from advertising code action support when

experimental.lsp.code_actions: false

...is set. We don't have experimental.lsp.code_actions set to true, but Flow's language server in older versions advertises support for code actions regardless (without specifying which ones) which causes the problem.

I think you understand the situation, but I wanted to make the problem more explicit in case there was a misunderstanding.