Allow JS with isolated modules #31483
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously legacy JS code was not allowed; it was required to use ES6 module syntax. Unfortunately, the check happens after parsing but before binding, and the commonjs module indicator isn't set until binding because it's not syntactically simple like the ES6 module indicator, which is set during parsing. So I decided that JS should be allowed during isolatedModules unconditionally. We're not going to be transforming it anyway.
|
@RyanCavanaugh You might have opinions about the new error wording. |
| { | ||
| "compilerOptions": { | ||
| "allowJs": true, | ||
| "noEmit": true, |
There was a problem hiding this comment.
What happens when noEmit is false?
There was a problem hiding this comment.
error saying that you can't overwrite index.js. The real example of this is from babel pipelines, where we recommended people use isolatedModules, so they use babel for emit, not typescript.
There was a problem hiding this comment.
But you could change it to emit into the outDir right? Would it affect in some way the emit that could happen?
There was a problem hiding this comment.
It shouldn't affect emit because we're not transforming js as far as I know. I pushed a commit with outDir replacing noEmit in the test.
sheetalkamat
approved these changes
May 23, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously legacy JS code was not allowed; it was required to use ES6 module syntax. Unfortunately, the check happens after parsing but before binding, and the commonjs module indicator isn't set until binding because it's not syntactically simple like the ES6 module indicator, which is set during parsing.
So I decided that JS should be allowed during isolatedModules unconditionally. We're not going to be transforming it anyway.
I also improved the error message, which mentions namespaces when most erroneous code it encounters will just be global scripts.
Fixes #29087