Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ranking unresolved imports with better relevance #340

Open
jay-w-opus opened this issue Aug 19, 2016 · 8 comments
Open

Ranking unresolved imports with better relevance #340

jay-w-opus opened this issue Aug 19, 2016 · 8 comments

Comments

@jay-w-opus
Copy link

There is a requirement that we want to find enough modules to import from. But it also brings up noise and brings down performance.

An idea to optimize performance: scan through the all the node_modules, but skip all its sub node_modules. It's rare to import anything from node_modules of a node_modules module, while those files are the most costy and noisy.

Ideas to rank the possible imports:

  1. Look at current file's sibling files. Imported modules referred by those files are likely more interested for current file
  2. To extend idea 1, we can use a daemon to construct a tree of import histograms. It will be a mirror of the workspace folder structure but the leaves will be the imported objects. When a file request a import keyword, then query through the tree to find all import leaves that has the matching name. Then use some algorithm to properly add weight or reduce weight based on number of usages and distance of those usages from current file.

I don't have specific algorithm in mind yet but it can be some heuristic approach.

@lencioni

@trotzig
Copy link
Collaborator

trotzig commented Aug 22, 2016

Interesting ideas. I think it definitely makes sense to peek into the node_modules folder for all your package dependencies.

When it comes to ranking and reordering, I worry that no ranking solution will be "good enough". Having to resolve an ambiguous import is already disrupting as it is, and having the selection list ordered in a different way might not help much. Using a simple strategy that people understand and that always results in the same order might actually be easier to use. Maybe @wincent can chime in here too? He's the author of Command-T and I've seen him deal with things like this a few times.

@jay-w-opus
Copy link
Author

Looking at other more sophisticated IDE such as IntelliJ, it is definitely a good user experience if the first choice is almost always the desirable result.

@trotzig
Copy link
Collaborator

trotzig commented Aug 23, 2016

That's a good point, and it makes sense. If we can make it "right" in 9/10 cases it makes sense to have a more sophisticated sorting strategy.

For your second point, the WatchmanFileCache class would be a good starting point. It already keeps a cache of all js files in your project.

@wincent
Copy link

wincent commented Aug 23, 2016

I don't have a horse in this race, but I do think a simple, opt-in heuristic might be worth looking at. One example of that would be having the ability to filter out non-top-level node modules. Ranking is much harder than blanket filtering though.

@trotzig
Copy link
Collaborator

trotzig commented Aug 23, 2016

Thanks @wincent for chiming in. At the moment we're not peeking in to any node modules folders, but we're discussing doing so for the top-level ones (those listed in package.json). Right now you can import a package dependency, but only by its name. You can make import-js import React from 'react'; but not import ReactMount from 'react/lib/ReactMount';.

🏁 🏇 🏇 🏇 🏇

@lencioni
Copy link
Collaborator

Yeah, I think we need to look at the files in each of the top-level dependencies. I'm a little worried about relevance, of the results, but maybe it will be okay.

One idea I have is to only consider files in the directories.lib directory specified in the package's package.json file. This would allow packages to opt in to this behavior which would help us avoid matching on random example files, documentation, test files, and other private modules.

@lencioni
Copy link
Collaborator

I think we need to include files in packages when matching, so I've opened #344 to track that. We can discuss and improve ranking of matches separately here.

@lencioni
Copy link
Collaborator

2.5.0 included a change that sorts the unresolved imports by path distance to the current file. I think there's more that we could do here, but this seems like a good and simple step in the right direction.

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

No branches or pull requests

4 participants