-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
--emitDeclarationOnly flag to enable declarations only output #20735
Conversation
src/compiler/program.ts
Outdated
if (options.noEmitJs && !options.declaration) { | ||
programDiagnostics.add(createCompilerDiagnostic(Diagnostics.Option_0_cannot_be_specified_without_specifying_option_1, "noEmitJs", "declaration")); | ||
} | ||
|
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.
you would also want to skip verifyEmitFilePath(emitFileNames.jsFilePath, emitFilesSeen);
down below when noEmitJs
is true
Kind ping regarding this @sheetalkamat Should I be adding any specific devs from typescript team to this PR? What's the usual process before a PR lands in the repo ? |
src/compiler/commandLineParser.ts
Outdated
@@ -231,6 +231,13 @@ namespace ts { | |||
category: Diagnostics.Basic_Options, | |||
description: Diagnostics.Do_not_emit_outputs, | |||
}, | |||
{ | |||
name: "noEmitJs", |
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.
I think the name should be --emitDeclarationsOnly
instead.
src/compiler/commandLineParser.ts
Outdated
name: "noEmitJs", | ||
type: "boolean", | ||
showInSimplifiedHelpView: true, | ||
category: Diagnostics.Basic_Options, |
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.
i would say this belongs into Advanced_Options
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.
Advanced_options don't show up as part of --help
or in default tsc --init. Doesn't it make sense that the option showed up there? I plan to enable allowJs + emitDeclarationsOnly in another PR which would help with lots of npm projects to auto-generate jsdoc to .d.ts declarations.
Puppeteer is one good example
src/compiler/diagnosticMessages.json
Outdated
@@ -2776,6 +2776,10 @@ | |||
"category": "Message", | |||
"code": 6013 | |||
}, | |||
"Do not emit js outputs.": { |
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.
Only emit declaration files.
src/server/server.ts
Outdated
@@ -980,7 +980,7 @@ namespace ts.server { | |||
}; | |||
|
|||
const ioSession = new IOSession(options); | |||
process.on("uncaughtException", err => { | |||
process.on("uncaughtException", (err: any) => { |
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.
why?
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.
Because it was failing compilation when doing a “gulp build” with an error.
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.
should work now. that was a @types/node issue.
merge when microsoft/TypeScript#20735 hits
@nojvek awesome job so far! The Puppeteer community would love some progress on this! ❤️ |
On it.
…On Wed, Jan 17, 2018 at 2:26 PM Bazyli Brzóska ***@***.***> wrote:
@nojvek <https://github.com/nojvek> the Puppeteer community would love
some progress on this! ❤️
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#20735 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA-JVO5w32fVMKrjX3ixMygNKGtq4l90ks5tLnOxgaJpZM4REbr->
.
|
@DanielRosenwasser and @sheetalkamat any concerns? |
Thank you @nojvek! Looking forward to the next PR 😊! |
For consistency with
It seems the pattern is to use singular What do you think ? I can send a rename PR |
humm.. good point.. we can change it to |
K will send a rename PR
…On Mon, Jan 29, 2018 at 4:05 PM Mohamed Hegazy ***@***.***> wrote:
humm.. good point.. we can change it to EmitDeclarationOnly
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#20735 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA-JVDl4kIDfOb339mxN7auA52aTAzFYks5tPly4gaJpZM4REbr->
.
|
rename PR up in #21651 |
* Add emitOnlyDeclarations flag * Fix name * verifyOptions checking logic * Passing tests * doJsEmitBaseline * Tests !!!
* --emitDeclarationsOnly flag to enable declarations only output (#20735) * Add emitOnlyDeclarations flag * Fix name * verifyOptions checking logic * Passing tests * doJsEmitBaseline * Tests !!! * Rename switch `--emitDeclarationsOnly` to `--emitDeclarationOnly` (#21651) * Rename `--emitDeclarationsOnly` to `--renameDeclarationOnly` * Rename test files
--declaration
with--allowJs
#7546master
branchjake runtests
locally