Skip to content

Fix #7397: Remove error checks for noEmit and out* compiler options combined. #7409

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 12, 2016
Merged

Fix #7397: Remove error checks for noEmit and out* compiler options combined. #7409

merged 2 commits into from
Mar 12, 2016

Conversation

anilanar
Copy link
Contributor

@anilanar anilanar commented Mar 6, 2016

Fixes #7397.

@msftclas
Copy link

msftclas commented Mar 6, 2016

Hi @anilanar, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

@DanielRosenwasser
Copy link
Member

@anilanar thanks for the PR. Can you add a set of tests in tests/cases/compiler to demonstrate the combination of noEmit with outFile, outDir, and declaration individually?

@anilanar anilanar force-pushed the allowOutWithNoEmit branch from dac8617 to cde06b6 Compare March 12, 2016 18:22
@anilanar
Copy link
Contributor Author

@DanielRosenwasser Done. I'm not sure if file names fit to a general philosophy (if any?) of naming them.

mhegazy
mhegazy reviewed Mar 12, 2016
View reviewed changes
src/compiler/program.ts
}
}
else if (options.allowJs && options.declaration) {
if (options.allowJs && options.declaration) {
programDiagnostics.add(createCompilerDiagnostic(Diagnostics.Option_0_cannot_be_specified_with_option_1, "allowJs", "declaration"));
Copy link
Contributor

Choose a reason for hiding this comment

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

&& !options.noEmit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mhegazy That would reflect the old behaviour. But I gave this a thought and I chose to do it this way because:

allowJs is incompatible with declaration. Then why does the compiler try to hide the error when noEmit is true? Maybe I should add && !options.noEmit and let this be discussed somewhere else.

@mhegazy
Copy link
Contributor

mhegazy commented Mar 12, 2016

Thanks!

mhegazy added a commit that referenced this pull request Mar 12, 2016
@mhegazy
Merge pull request #7409 from anilanar/allowOutWithNoEmit
74e498d 
Fix #7397: Remove error checks for noEmit and out* compiler options combined.
@mhegazy mhegazy merged commit 74e498d into microsoft:master Mar 12, 2016
@anilanar anilanar deleted the allowOutWithNoEmit branch March 12, 2016 20:35
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@anilanar @msftclas @DanielRosenwasser @mhegazy