Skip to content

Conversation

nojvek
Copy link
Contributor

@nojvek nojvek commented Dec 16, 2017

@nojvek nojvek changed the title Adding noEmitJs flag to enable declarations only output noEmitJs flag to enable declarations only output Dec 16, 2017
if (options.noEmitJs && !options.declaration) {
programDiagnostics.add(createCompilerDiagnostic(Diagnostics.Option_0_cannot_be_specified_without_specifying_option_1, "noEmitJs", "declaration"));
}

Copy link
Member

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

@nojvek
Copy link
Contributor Author

nojvek commented Jan 3, 2018

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 ?

description: Diagnostics.Do_not_emit_outputs,
},
{
name: "noEmitJs",
Copy link
Contributor

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.

name: "noEmitJs",
type: "boolean",
showInSimplifiedHelpView: true,
category: Diagnostics.Basic_Options,
Copy link
Contributor

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

Copy link
Contributor Author

@nojvek nojvek Jan 21, 2018

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

"category": "Message",
"code": 6013
},
"Do not emit js outputs.": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Only emit declaration files.


const ioSession = new IOSession(options);
process.on("uncaughtException", err => {
process.on("uncaughtException", (err: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

kevlened added a commit to kevlened/str2buf that referenced this pull request Jan 6, 2018
@niieani
Copy link

niieani commented Jan 17, 2018

@nojvek awesome job so far! The Puppeteer community would love some progress on this! ❤️

@nojvek
Copy link
Contributor Author

nojvek commented Jan 17, 2018 via email

@nojvek nojvek changed the title noEmitJs flag to enable declarations only output --emitDeclarationsOnly flag to enable declarations only output Jan 23, 2018
@nojvek
Copy link
Contributor Author

nojvek commented Jan 23, 2018

@mhegazy want to take another pass ?

@niieani - Unfortunately this PR doesn't enable emitting declarations from a JS project. I'll start work on it as soon as this PR lands.

@mhegazy
Copy link
Contributor

mhegazy commented Jan 24, 2018

@DanielRosenwasser and @sheetalkamat any concerns?

@mhegazy mhegazy merged commit afc588e into microsoft:master Jan 25, 2018
@niieani
Copy link

niieani commented Jan 26, 2018

Thank you @nojvek! Looking forward to the next PR 😊!

@nojvek
Copy link
Contributor Author

nojvek commented Jan 29, 2018

@mhegazy @sheetalkamat

// @declaration: true
// @emitDeclarationsOnly: true

For consistency with declaration: true flag, does it make sense to call this new flag emitDeclarationOnly ?

        declaration?: boolean;
        emitDeclarationsOnly?: boolean;
        declarationDir?: string;

It seems the pattern is to use singular *declaration*.

What do you think ?

I can send a rename PR

@mhegazy
Copy link
Contributor

mhegazy commented Jan 30, 2018

humm.. good point.. we can change it to EmitDeclarationOnly

@nojvek
Copy link
Contributor Author

nojvek commented Jan 30, 2018 via email

@mhegazy
Copy link
Contributor

mhegazy commented Feb 5, 2018

rename PR up in #21651

@mhegazy mhegazy changed the title --emitDeclarationsOnly flag to enable declarations only output --emitDeclarationOnly flag to enable declarations only output Feb 5, 2018
mhegazy pushed a commit that referenced this pull request Feb 5, 2018
* Add emitOnlyDeclarations flag

* Fix name

* verifyOptions checking logic

* Passing tests

* doJsEmitBaseline

* Tests !!!
mhegazy added a commit that referenced this pull request Feb 6, 2018
* --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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

4 participants