Skip to content

Search for node_modules in parent directories when getting type roots. #10670

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

Merged
4 commits merged into from
Sep 2, 2016

Conversation

ghost
Copy link

@ghost ghost commented Sep 1, 2016

Fixes #10617
Note that this only gets the nearest node_modules. It would be easy to modify this code to search for as many as possible rather than just taking the first one; would we want that?
Also, this does not get globally installed node_modules. This has a different location on different people's machines, although we could just include all possible locations and check of the ones that don't exist.
Also, @vladima is there a reason we have directoryExists optional but fileExists mandatory?

@yortus
Copy link
Contributor

yortus commented Sep 2, 2016

@Andy-MS could we make the behaviour consistent with the compiler's existing moduleResulution: node implementation? That's already present in the compiler and it searches up multiple levels (if neccessary) which matches runtime behaviour (except I think it doesn't look for global node_modules).

The existing 'searching up' implementation is in program.ts here.

I understand this is a slightly different thing, because you are not searching for a specific module, but looking for whatever you can find in a @types directory. But it's more consistent with runtime behaviour if you keep searching up and accumulate all the @types/X typings you find.

If the compiler does not search up multiple levels for @types, there are two problems that would be surprising for users:

  1. it will fail to compile programs that are valid/work fine at runtime.
  2. it will be internally inconsistent in that something like import * as ts from 'typescript' will search up multiple levels to find the tsc typings, but import * as fs from 'fs' will not search up multiple levels for the node typings.

@mhegazy
Copy link
Contributor

mhegazy commented Sep 2, 2016

👍

@mhegazy
Copy link
Contributor

mhegazy commented Sep 2, 2016

please port these to release-2.0 branch as well.

@fletchsod-developer
Copy link

Since this is fixed in 2.0 branch. Question is which release version at the earliest contain this fix?

@ghost
Copy link
Author

ghost commented Oct 18, 2016

@fletchsod-developer Works for me in 2.0.3. This isn't in 2.0.2 as that was published in before this change was made.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 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.

4 participants