facebook / flow

Adds static typing to JavaScript to improve developer productivity and code quality.
https://flow.org/
MIT License
22.08k stars 1.86k forks source link

Add Auto Import functionality to LSP #7662

Closed rattrayalex-stripe closed 2 years ago

rattrayalex-stripe commented 5 years ago

Proposal

Blocks https://github.com/flowtype/flow-for-vscode/issues/181

Opening per recommendation from @vicapow in https://github.com/flowtype/flow-for-vscode/issues/181#issuecomment-484987794

VSCode has "Auto Import" functionality for JS/TS, using the TS LSP.

This is a great feature and many of our engineers complain that Flow does not support it. It is a leading example our engineers offer when asking to move from Flow to TS for "better tooling".

Use case

Developer writes code referencing a class or function from another file which they have not yet imported. They press save and the class or function is automatically added to the imports in their file (at least, assuming it is an unambiguous import).

vicapow commented 5 years ago

Thanks for creating this ticket, @rattrayalex-stripe! This is also something folks have brought up for us internally at Uber. I can try to get to this, if I get some time but no promises :P

VSCode has "Auto Import" functionality for JS/TS, using the TS LSP.

To clarify, TypeScript's editor support is implemented with the tsserver, which isn't implemented with the Language Server Protocol. It may always be the case TS + tsserver has features that would require updates to the LSP spec. ie., https://github.com/Microsoft/vscode-languageserver-node/issues/471

noppa commented 5 years ago

This would be great! Automatic imports are a huge productivity booster and make it easier to structure code in modules with small functions in them, as opposed to a more class-heavy API design where semi-related functionality is glued together in bloated containers just to make it easier to use the methods in them.

One problem that might arise with this is module name mappings. Flow allows regex -> string mapping between an import name and the actual file path. So if I have, say,


module.name_mapper='^\$root/\(.*\)$' -> '<PROJECT_ROOT>/\1'

I can then import functions from index.js using


import * as foo from '$root/index.js'

or even


import * as foo from '$root'

(edit: there's a minor mistake in the regex that makes this example fail, but you get the point)

Ideally, auto import should also use these forms to refer to the files, because otherwise I'm just going to have to clean up the paths after it. But mapping from the known actual file path to whatever form my regex describes can be difficult (impossible?) to do.

PierreCapo commented 5 years ago

Is there any roadmap for this feature ? I'm experiencing the same issue than OP: auto-import is a killer feature and a lot of devs from my company complain it is not present in flow compared to TypeScript. Thanks for the good work by the way !

villesau commented 5 years ago

Looks like @jedwards1211 has been working on workaround called dude-wheres-my-module for that: https://github.com/flowtype/flow-for-vscode/issues/181#issuecomment-524127652

jedwards1211 commented 4 years ago

Hi everyone, I just published a VSCode extension for dude-wheres-my-module!

https://github.com/jedwards1211/vscode-dude-wheres-my-module

noppa commented 3 years ago

Flow version 0.143 introduced autoimports feature behind autoimports=true flowconfig option :tada:

If anyone else happens to be stuck in version 0.142 because of the "classic mode"/"types first mode" change, I have a fork of version 0.142 with some of @mroch's great work on LSP features cherry-picked to it, autoimports included.

jedwards1211 commented 3 years ago

@noppa That's cool!

I want to point out a helpful feature of dude-wheres-my-module that's sorely lacking in typescript, and I bet also the new flow autoimports system.

Imagine that you have many React component modules that export type Props = .... Since their names would clash you'd probably be renaming one of them if you import it, like:

import Button, { type Props as ButtonProps } from './Button'

If you have that in module A, and you type ButtonProps in a new module B, it's helpful if the autoimport system can suggest the above import to bind it.

Other thoughts:

jedwards1211 commented 3 years ago

@noppa btw, autoimports is missing from the options docs

noppa commented 3 years ago

@jedwards1211 Yeah, good points. It also currently doesn't handle module name mappings, I'm using eslint-plugin-import-alias and import/no-duplicates with autofix enabled to mitigate that.

I've used dude-wheres-my-module while waiting for this feature to land in Flow and it's definitely still a viable alternative with all its configuration options and smart features like that.

In the long term, I feel like having this feature integrated with the type system could have some benefits over separate solutions. I'm imagining a feature where if I have something like

// some other file
export async function findUserByUsername(username: string): Promise<void | User> {
 ...
}

// file I'm editing
async function modifyUser({username}: {username: string})  {
  const user: void | User =|

it could maybe suggest findUserByUsername at that point, given that there's an unused string variable in scope and the return types match. Or, if we ever get the pipeline operator, it could look at the type coming from LHS of |> and suggest functions that take that type in RHS.

I'm kind of jealous of OOP people who can just write value.| and the IDE will neatly list all the operations that you can do with that value. I wish we could someday have that kind of method discovery built for plain old functions that would basically answer the question "what are some operations that I can do with this value".

Of course, Flow doesn't currently do anything of the sort even for local functions/values, so I don't expect this kind of stuff to land anytime soon, if ever. But hey, one can dream.


noppa btw, autoimports is missing from the options docs

Yep, the option should probably be added to docs. Might do a PR for it later but I'm not in the team so there's not much else I can do about that. In my fork the option isn't there (as I'm trying to keep compatibility with 142), the feature is just enabled by default.

mroch commented 3 years ago

@noppa I'd love to build a type-based search like Hoogle! I think that's what we'd need to power that kind of autocomplete. even more wild, imagine searching for a function by example, like in Smalltalk: input 'foo', output 'FOO' would find 'foo'.toUpperCase() :-O

mroch commented 3 years ago

Yep, the option should probably be added to docs.

it wasn't documented yet because it was very experimental. the idea is to make it on by default and remove the option, but would definitely appreciate a PR to document it (as still experimental) in the meantime!

jedwards1211 commented 3 years ago

I'm kind of jealous of OOP people who can just write value.| and the IDE will neatly list all the operations that you can do with that value

The VSCode Flow extension does this now, right? Though not the additional things you're proposing

jedwards1211 commented 3 years ago

@mroch if you can point me to the PR(s) where autoimports landed, I can try to learn the details about it, what features there are, etc. without asking y'all too many questions, and then maybe make a PR

noppa commented 3 years ago

@jedwards1211 Yes, if you have methods in an object, e.g user.getFullName() Flow will autocomplete those for you. However, that won't help much if your codebase is structured in a more functional style, so that you'd actually want getFullName(user). My comment was more of throwing around long-term ideas (or wishful thinking) of features that could someday help with that kind of functional style codebases.

jedwards1211 commented 3 years ago

If I change a widely-used type and the Flow server has to spend 30s on typechecking, will import suggestions be completely blocked for those 30s? Or is the suggested import index an independent system that can respond quickly even while rechecking types?

SamChou19815 commented 2 years ago

Auto import has been implemented a while ago.