-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Type check all files when emitting a file under -out #3094
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
Conversation
// This is because in the -out scenario all files need to be emitted, and therefore all | ||
// files need to be type checked. And the way to specify that all files need to be type | ||
// checked is to not pass the file to getEmitResolver. | ||
let emitResolver = getDiagnosticsProducingTypeChecker().getEmitResolver(options.out ? undefined : sourceFile); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch. I'm wondering if there's a way to make this less error prone (though nothing is coming to mind). To some extent, what is weird is a model where a source file is passed at all to emit when you have -out. I mean, what does it mean to say "emit foo.ts" or "emit bar.ts" when you have "--out"? There's no effective difference, and it probably makes more sense for this to not be supported, or for the caller to have to say "emit all". (i'm just speaking out loud. i don't have an actual proposal).
👍 |
Ok. So the managed side already distinguishes the idea between asking to get the emit output whne you have --out or not. So i think a better solution would be to make it an error to ask for emit output for a single file when you have --out. Instead, if you have --out, you just call getEmitResult with no source file specified. If you want the emit output for a single file, and you don't have --out, then you cna ask for it, and it should work. Of course, this would be a breaking change. So we'd need to do this through some sort of getEmitOutput2 API . |
@CyrusNajmabadi Looking at the logic in emitFiles that decides which files to emit, the choice of whether or not a file is specified does not correspond one-to-one with whether the compiler options include -out. The most salient example is -out with external modules. You can have -out, but call getEmitOutput on an external module, and it will only emit that module. That seems like a meaningful behavior, and the most obvious way to signify it is by passing a source file. Another example (less interesting) is if you getEmitOutput for a declaration file when -out is specified, which actually does not emit anything. This one doesn't matter as much because callers can just not call getEmitOutput for a declaration file. In fact, I could see making it an error to call getEmitOutput on a declaration file. But I think for the external module case, passing a source file to getEmitOutput is the best way to emit it. |
03f8ea4
to
0401553
Compare
Ah true. So your approach makes a ton of sense, and is a very nice, lightweight way to get non-broken behavior. 👍 |
Note: this shoudl be something we should be able to make a fourslash test for. Just a simple case with two files, with appropriate options set, and we ask for getEmitOutput for one file and get the wrong result. |
It does have a fourslash test. It's part of the review. |
Type check all files when emitting a file under -out
Yes. I am blind. |
Since it is necessary to type check a source file before you emit it, it is necessary to type check all files before emitting -out.
Thanks @vladima for your help on this fix. It fixes #2061.