Don't suggest import completions for /a/node_modules if we're in /b#19928
Conversation
5d07e28 to
b6d3c88
Compare
| // @Filename: package.json | ||
| //// { "dependencies": { "package-name": "latest" } } | ||
|
|
||
| // @Filename: node_modules/package-name/node_modules/package-name2/bin/lib/libfile.d.ts |
There was a problem hiding this comment.
Don't think we should have been getting imports from here -- it's fishy to directly import a dependency of a dependency.
There was a problem hiding this comment.
Agreed it's fishy, but this tests seems it's a regression from something that can be done.
I recommend leaving the test in place to check for any regressions.
There was a problem hiding this comment.
I deleted it file because the test started failing, so it's an intentional regression -- I really think we shouldn't have been doing this.
This test was added in #17302 -- @minestarks is this OK to delete?
There was a problem hiding this comment.
yep seems fine to quit doing this. You could just change the expectation of the test case though...
c9872f8 to
3eb84af
Compare
| // @Filename: package.json | ||
| //// { "dependencies": { "package-name": "latest" } } | ||
|
|
||
| // @Filename: node_modules/package-name/node_modules/package-name2/bin/lib/libfile.d.ts |
There was a problem hiding this comment.
Agreed it's fishy, but this tests seems it's a regression from something that can be done.
I recommend leaving the test in place to check for any regressions.
9b74bd3 to
5c0c390
Compare
|
@armanio123 @minestarks Please re-review |
Fixes #19878
Relative imports into
node_modulesare bad, especially../../../../../../../../../../../../../../Library/Caches/typescript/2.6/node_modules/@types/gulp-util.