Skip to content

Fix service host interface to support ts2.5 - #366

Merged
blakeembrey merged 2 commits into
TypeStrong:masterfrom
cevek:fix-ts2.5
Jun 21, 2017
Merged

blakeembrey merged 2 commits into
TypeStrong:masterfrom
cevek:fix-ts2.5

Conversation

@cevek

@cevek cevek commented Jun 20, 2017

Copy link
Copy Markdown
Contributor

In typescript 2.5 language service host needs to implement fileExists, readFile and readDirectory to be able to correctly resolve the modules (otherwise only ambient modules would be resolved)
Described here https://github.com/Microsoft/TypeScript/blame/master/src/services/types.ts#L166

@coveralls

coveralls commented Jun 20, 2017 •

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 86.538% when pulling 09c7482 on cevek:fix-ts2.5 into 28697b1 on TypeStrong:master.

Comment thread src/index.ts
directoryExists: directoryExists,
fileExists: ts.sys.fileExists,
readFile: ts.sys.readFile,
readDirectory: ts.sys.readDirectory,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have existing implementations of two of these functions, should we just use those?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blakeembrey Fixed

@coveralls

coveralls commented Jun 21, 2017 •

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 86.538% when pulling a99486f on cevek:fix-ts2.5 into 28697b1 on TypeStrong:master.

@johnnyreilly

Copy link
Copy Markdown
Member

@blakeembrey - do you know if we need to make a similar change to ts-loader here: https://github.com/TypeStrong/ts-loader/blob/master/src/servicesHost.ts#L34 ?

@blakeembrey

Copy link
Copy Markdown
Member

@johnnyreilly I would think so, yes. You can just use the ts.sys functions though.

@johnnyreilly

Copy link
Copy Markdown
Member

Thanks @blakeembrey - I've tested and it looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants