Fix type reference directive resolution #936
Conversation
| resolveTypeReferenceDirective( | ||
| directive, | ||
| containingFile, | ||
| _redirectedReference |
There was a problem hiding this comment.
This wasn't related to the particular repro provided, but it seemed like a place that might be broken for even edgier edge cases
| @@ -512,16 +518,17 @@ function makeResolveTypeReferenceDirective( | |||
| | undefined | |||
| ): ResolveTypeReferenceDirective { | |||
| if (customResolveTypeReferenceDirective === undefined) { | |||
| return (directive: string, containingFile: string) => | |||
| return (directive, containingFile, redirectedReference) => | |||
There was a problem hiding this comment.
Contextual parameter types FTW 😄
|
Heya @andrewbranch! thanks for this! Will take a look now |
| directoryExists: compiler.sys.directoryExists | ||
| directoryExists: compiler.sys.directoryExists, | ||
| getCurrentDirectory: compiler.sys.getCurrentDirectory, | ||
| getDirectories: compiler.sys.getDirectories | ||
| }; |
There was a problem hiding this comment.
So if I understand this correctly, the fix is the implementation of these:
directoryExists: compiler.sys.directoryExists,
getCurrentDirectory: compiler.sys.getCurrentDirectory,
getDirectories: compiler.sys.getDirectories
?
That's interesting. I wish it was more "fail fast" here if it actually needed these...
There was a problem hiding this comment.
Yep, specifically getCurrentDirectory. I agree. I'll suggest something like this to the team.
This is very interesting. Completely agree
Yeah - agree! Could you update the |
|
Done 👍 |
|
Hold on the release, I've got another fix coming |
|
I'm going to mercifully let CI off the hook of finishing testing my updates to the CHANGELOG—save some electricity, you know |
Fixes #934
Fixes #919
There was really no way to know this without debugging, but getting default typeRoots for a project doesn't work if the module resolution host doesn't implement
getCurrentDirectory(). Arguably the language service API shouldn’t treat it as optional, or should emit a warning if it’s missing, or fall back to the default fromts.sys(maybe I'll bring this up to Sheetal, but it certainly wouldn't land until 3.6), but I think a good policy for ts-loader would be to implement all members of any kind of Host, especially if it’s as easy a just including things fromts.sys. It is kind of misleading because there are other places (I thinkCompilerHostis an example) where missing optional members get filled in with good defaults.