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

Merged
merged 6 commits into from Jan 25, 2018

Conversation

Projects
None yet
4 participants
@nojvek
Contributor

nojvek commented Dec 16, 2017

  • Fixes #15158 and #1866. Part of fix for #7546
  • Code is up-to-date with the master branch
  • You've successfully run jake runtests locally
  • You've signed the CLA
  • There are new or updated unit tests validating the change

@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"));
}

This comment has been minimized.

@sheetalkamat

sheetalkamat Dec 18, 2017

Member

you would also want to skip verifyEmitFilePath(emitFileNames.jsFilePath, emitFilesSeen); down below when noEmitJs is true

@nojvek

This comment has been minimized.

Contributor

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 ?

@@ -231,6 +231,13 @@ namespace ts {
category: Diagnostics.Basic_Options,
description: Diagnostics.Do_not_emit_outputs,
},
{
name: "noEmitJs",

This comment has been minimized.

@mhegazy

mhegazy Jan 5, 2018

Contributor

I think the name should be --emitDeclarationsOnly instead.

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

This comment has been minimized.

@mhegazy

mhegazy Jan 5, 2018

Contributor

i would say this belongs into Advanced_Options

This comment has been minimized.

@nojvek

nojvek Jan 21, 2018

Contributor

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

@@ -2776,6 +2776,10 @@
"category": "Message",
"code": 6013
},
"Do not emit js outputs.": {

This comment has been minimized.

@mhegazy

mhegazy Jan 5, 2018

Contributor

Only emit declaration files.

@@ -980,7 +980,7 @@ namespace ts.server {
};

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

This comment has been minimized.

@mhegazy

mhegazy Jan 5, 2018

Contributor

why?

This comment has been minimized.

@nojvek

nojvek Jan 5, 2018

Contributor

Because it was failing compilation when doing a “gulp build” with an error.

This comment has been minimized.

@mhegazy

mhegazy Jan 6, 2018

Contributor

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

This comment has been minimized.

niieani commented Jan 17, 2018

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

@nojvek

This comment has been minimized.

Contributor

nojvek commented Jan 17, 2018

@nojvek nojvek force-pushed the nojvek:noEmitJs branch from 49c0f26 to e34b4fb Jan 23, 2018

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

@nojvek

This comment has been minimized.

Contributor

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

This comment has been minimized.

Contributor

mhegazy commented Jan 24, 2018

@DanielRosenwasser and @sheetalkamat any concerns?

@mhegazy mhegazy merged commit afc588e into Microsoft:master Jan 25, 2018

5 checks passed

TypeScript Test Run typescript_node.6 Build finished.
Details
TypeScript Test Run typescript_node.8 Build finished.
Details
TypeScript Test Run typescript_node.stable Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details
@niieani

This comment has been minimized.

niieani commented Jan 26, 2018

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

@nojvek

This comment has been minimized.

Contributor

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

This comment has been minimized.

Contributor

mhegazy commented Jan 30, 2018

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

@nojvek

This comment has been minimized.

Contributor

nojvek commented Jan 30, 2018

@mhegazy

This comment has been minimized.

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 added a commit that referenced this pull request Feb 5, 2018

--emitDeclarationsOnly flag to enable declarations only output (#20735)
* Add emitOnlyDeclarations flag

* Fix name

* verifyOptions checking logic

* Passing tests

* doJsEmitBaseline

* Tests !!!

mhegazy added a commit that referenced this pull request Feb 6, 2018

Port --emitDeclarationOnly flag support to release-2.7 (#21654)
* --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

@mhegazy mhegazy referenced this pull request Feb 6, 2018

Closed

Add new flag support for TS 2.7.2 #21657

3 of 3 tasks complete

@Microsoft Microsoft locked and limited conversation to collaborators Jul 3, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.