Don't lose the this reference for compilerHost methods. #1546
Conversation
| getCommonSourceDirectory: program.getCommonSourceDirectory, | ||
| getCompilerOptions: program.getCompilerOptions, | ||
| getCurrentDirectory: compilerHost.getCurrentDirectory, | ||
| getCurrentDirectory: () => compilerHost.getCurrentDirectory(), | ||
| getNewLine: compilerHost.getNewLine, |
There was a problem hiding this comment.
Sounds a little ridiculous, but maybe getNewLine should do this too, at least for the sake of consistency.
I kind of doubt program methods would need it, since we're the ones producing values of the type Program, but it might be useful for those producing a custom parse tree. This happens to be a scenario we're seeing people want as well.
There was a problem hiding this comment.
Oh right, I missed that one - my host doesn't use this inside getNewLine() so I didn't catch it.
I'll change the program ones too.
|
👍 |
|
Rebased onto latest master (no changes). |
|
👍 |
|
Sorry this totally fell off the radar. @Arnavion sorry for the delay, but can you update this PR |
|
I've updated it, but I'm not in a position to test it works right now. I've also undone the change for the program methods, since now the program is internally generated by both tsc and services. (In the original change the program was a parameter to createEmitHostFromProgram, so there was the possibility a user could pass in their own instance). I see that EmitHost.writeFile and CompilerHost.writeFile are now typed to the same interface, so there'd be no potential breakage in using bind() instead of writing the parameters explicitly (no possibility of updating one and forgetting to update the other, and then not catching it because the converter uses bind). Do you want me to do that? |
|
yeah you are right. I will close the issue then. thanks! and sorry for the delay. |
|
Err, why? |
|
Sorry. I think i need more coffee. I personally like your change now better than bind. |
|
Would be nice to see PRs get merged rather quickly, if there are no issues. It encourages community to contribute to the project. |
|
@jasonwilliams200OK agreed; sorry about the delay @Arnavion. |
Don't lose the this reference for compilerHost methods.
Fixes #1545