sourcegraph / sourcegraph

Code AI platform with Code Search & Cody
https://sourcegraph.com
Other
9.89k stars 1.2k forks source link

Skip local dependencies for NPM packages #31137

Open varungandhi-src opened 2 years ago

varungandhi-src commented 2 years ago

It would be good to figure out a way when a dependency is local (vs based on an NPM package in the registry) vs when it comes from the package registry. This affects both correctness and ergonomics:

I don't think this information is communicated in the LSIF dump today, so we would need to fix that first.

github-actions[bot] commented 2 years ago

Heads up @macraig - the "team/code-intelligence" label was applied to this issue.

varungandhi-src commented 2 years ago

On studying the LSIF, I guess it does seem to be specified in an indirect way to some extent. Here is a subset of output from the client/web/ dump.

{"id":9,"type":"vertex","label":"packageInformation","name":"@sourcegraph/web","manager":"npm","version":"1.10.1"}
{"id":13802,"type":"vertex","label":"packageInformation","name":"@sourcegraph/extension-api-types","manager":"npm","version":"2.1.0","repository":{"type":"git","url":"https://github.com/sourcegraph/sourcegraph","directory":"client/extension-api-types"}}
{"id":50382,"type":"vertex","label":"packageInformation","name":"sourcegraph","manager":"npm","version":"25.5.0","repository":{"type":"git","url":"https://github.com/sourcegraph/sourcegraph","directory":"client/extension-api"}}
{"id":1208,"type":"vertex","label":"packageInformation","name":"@types/lodash","manager":"npm","version":"4.14.167","repository":{"type":"git","url":"https://github.com/DefinitelyTyped/DefinitelyTyped.git","directory":"types/lodash"}}
{"id":2462,"type":"vertex","label":"packageInformation","name":"rxjs","manager":"npm","version":"6.6.3","repository":{"type":"git","url":"https://github.com/reactivex/rxjs.git"}}
{"id":50415,"type":"vertex","label":"packageInformation","name":"@sourcegraph/common","manager":"npm","version":"0.0.1"}

Some observations:

So checking for the repository field would be a partial solution. I suspect it's still not fully correct because it would make sense for the build to be using the local client/extension-api-types rather than a version downloaded from NPM.

macraig commented 2 years ago

@varungandhi-src is this still relevant with scip-typescript?

varungandhi-src commented 2 years ago

Yes, this is still applicable, it is a sub-task of the linked issue about different dependency kinds. Right now, we don't treat local dependencies differently, but we probably should.