Skip to content

In forEachExternalModule, skip node_modules beginning with _.#19910

Closed
ghost wants to merge 1 commit into
masterfrom
importNameCodeFix_nodeModulesUnderscore
Closed

In forEachExternalModule, skip node_modules beginning with _.#19910
ghost wants to merge 1 commit into
masterfrom
importNameCodeFix_nodeModulesUnderscore

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Nov 10, 2017

Fixes #19893
This is probably not the right solution -- why are we including everything in node_modules even when they were never imported before?

@ghost ghost force-pushed the importNameCodeFix_nodeModulesUnderscore branch from 29d6dd4 to 4ddc27b Compare November 10, 2017 16:13
@ghost ghost force-pushed the importNameCodeFix_nodeModulesUnderscore branch from 4ddc27b to e7d1095 Compare November 10, 2017 16:17
@weswigham
Copy link
Copy Markdown
Member

weswigham commented Nov 10, 2017

why are we including everything in node_modules even when they were never imported before?

Because our default behavior is to search for files which many contain global declarations, like @types/node. This is canonical, and the way to disable it is to specify types: [] (or an actual list of explicit refs) in your tsconfig. We probably shouldn't special-case this, since underscore-prefixed-versioned-libraries-alongside-normal-ones is not what a standard npm environment does, and it can be dealt with via configuration.

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Nov 10, 2017

this seems rather arbitrary solution.. there has to be a more structured/holistic approach to addressing this problem. i am not sure i understand the behavior of cnpm to be able to have an educated discussion, so i will need to readup and then we can talk.

@ghost
Copy link
Copy Markdown
Author

ghost commented Nov 10, 2017

I think what matters is that there's junk in mode_modules that's automatically included. Not really cnpm specific since there's usually lots of junk in node_modules anyway. See #19893 (comment) from @sheetalkamat

@sheetalkamat
Copy link
Copy Markdown
Member

@andy the _rax included in the program is not because everything in node_modules is auto included.. That portion works well wherein when we get base file names for tsconfig.json we wouldnt include node_modules. The file when imports rax it includes the _raxXXXX thing because in the node_modules rax is npm link to that folder... And hence it shows up in the program files.. If you look at the tsserver project files, it wont have any other modules with _ as the prefix. I dont think the current fix is right solution

@ghost ghost closed this Dec 11, 2017
@ghost ghost deleted the importNameCodeFix_nodeModulesUnderscore branch December 11, 2017 16:34
@ghost
Copy link
Copy Markdown
Author

ghost commented Dec 11, 2017

Replaced by #20395

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants