Add module: es2022 by saschanaz · Pull Request #44656 · microsoft/TypeScript (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation9 Commits1 Checks0 Files changed
Conversation
This file contains hidden or 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
[ Show hidden characters]({{ revealButtonHref }})
| @@ -184,7 +184,7 @@ namespace ts { |
|---|
| file: undefined, |
| start: 0, |
| length: 0, |
| messageText: "Argument for '--module' option must be: 'none', 'commonjs', 'amd', 'system', 'umd', 'es6', 'es2015', 'esnext'.", |
| messageText: "Argument for '--module' option must be: 'none', 'commonjs', 'amd', 'system', 'umd', 'es6', 'es2015', 'es2020', 'es2022', 'esnext'.", |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting to see this hasn't failed. I don't think this even runs 🤔
Should this function return ModuleKind.ES2020 and ES2022 following the target version? It seems #33893 missed this, and I wonder this needs to be fixed.
| export function getEmitModuleKind(compilerOptions: {module?: CompilerOptions["module"], target?: CompilerOptions["target"]}) { |
|---|
| return typeof compilerOptions.module === "number" ? |
| compilerOptions.module : |
| getEmitScriptTarget(compilerOptions) >= ScriptTarget.ES2015 ? ModuleKind.ES2015 : ModuleKind.CommonJS; |
| } |
PR seems kind of outdated.
I'll rebase once anyone reviews this (or a maintainer requests a rebase).
This is going to look a weeee bit different after a merge since the module: node changes touch many of the same lines, but initially it looks ok.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This little looks fine to me - @DanielRosenwasser do we want this in for the beta?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this seems reasonable.
What could be the default option for this and the highest version, I guess is none and esnext?