Conversation
|
|
||
| const defaultLibrarySearchPaths = <Path[]>[ | ||
| "types/", | ||
| "node_modules/", |
There was a problem hiding this comment.
Is it safe to remove this now?
There was a problem hiding this comment.
cast to Path - yes, I'll drop it. As for defaultLibrarySearchPaths - no, they are still used here
| } | ||
| }, | ||
| { | ||
| name: "traceModuleResolution", |
There was a problem hiding this comment.
Is this not a breaking change? Should you flag these as experimental?
There was a problem hiding this comment.
this flag was planned to initially appear in 2.0 which was not officially released yes so no, technically it is not a breaking change. There are few folks who might already use this flag, I'll ping them once this PR is approved. Re experimental - I don't think this is applicable here.
|
👍 |
src/compiler/program.ts
Outdated
| return options.typesSearchPaths; | ||
| } | ||
| return options.configFilePath | ||
| ? [getDirectoryPath(options.configFilePath)].concat(defaultLibrarySearchPaths) |
There was a problem hiding this comment.
why do we need the directory of the tsconfig.json?
src/compiler/program.ts
Outdated
| } | ||
| } | ||
|
|
||
| function getEffectiveTypesPrimarySearchPaths(options: CompilerOptions): string[] { |
There was a problem hiding this comment.
do not think we need the function here. the name is not much simpler than the actual implementation. consider inlining the function.
|
We should also add a new property to tsconfig.json |
|
No, it should be included |
| "code": 6104 | ||
| }, | ||
| "Expected type of 'typings' field in 'package.json' to be 'string', got '{0}'.": { | ||
| "Expected type of '{0}' field in 'package.json' to be 'string', got '{1}'.": { |
There was a problem hiding this comment.
Why not generalize this to take the expected type as well?
…as a root when computing primary locations
|
👍 |
| let fileProcessingDiagnostics = createDiagnosticCollection(); | ||
| let skipDefaultLib = options.noLib; | ||
| const programDiagnostics = createDiagnosticCollection(); | ||
| const currentDirectory = host.getCurrentDirectory(); |
There was a problem hiding this comment.
This line breaks the optional nature of the host argument
|
This PR breaks the optional nature of the host argument in |
|
Oops, good catch! Thanks @jbrantly for noticing this, fix will be out shortly |
6 participants
This PR continues the work started in #7549