Allow just generation of declarations, also allowJs #15911

nojvek · 2017-05-17T17:55:16Z

Scenario 1: Auto-Generate typings for a pure js project to be bundled with npm module
Scenario 2: Just emit typings for a project that is a hybrid of ts and js

Currently typings for js projects are manually hand maintained. A ton of typings are out of date with the actual code. Typings are kind of documentation and if they are separate from code, they will eventually get out of sync.

with --checkJs, JS lib authors get extra benefit of validating code with jsdoc typings. Being able to auto generate just .d.ts from their existing code would make life easier. This can be done as part of the build step.

Basically if declarations:true and noEmit:true, declarations should still be generated. If allowJs:true, then declarations should also be generated for the js files.

Proposal:

{
    compilerOptions: {
        "outFile": "lib/main.d.ts",
        "declarations": true,
        "allowJs": true,
        "checkJs": true, // optional
        "noEmit": true,
    },
    include: [
        "src/**.js"
    ]
}

This would just generate a single .d.ts file from the project that you can add as part of package.json. No hand maintenance.

{
    "name": "awesome",
    "author": "Awesome Industries",
    "version": "1.0.0",
    "main": "./lib/main.js",
    "types": "./lib/main.d.ts"
}

There is an associated issue that is labelled
Code is up-to-date with the master branch
You've signed the CLA
There are new or updated unit tests validating the change
You've successfully run gulp runtests locally (Need to fix some tests - WIP)

Fixes #7546 #1866

DanielRosenwasser · 2017-05-19T00:19:27Z

src/compiler/program.ts

@@ -904,7 +904,13 @@ namespace ts {
            let declarationDiagnostics: Diagnostic[] = [];

            if (options.noEmit) {
-                return { diagnostics: declarationDiagnostics, sourceMaps: undefined, emittedFiles: undefined, emitSkipped: true };
+                // Allow only emitting of declarations
+                if (options.declaration) {


Hm, this is interesting. The user wrote noEmit, but you're still emitting .d.ts files. Is there a better mechanism to surface this behavior? @mhegazy @bowdenk7

I think it makes sense for declaration:true to override noEmit. Since it's something you have to do explicitly. I.e just having noEmit will emit nothing. If you really really want just declarations which is a valid use case then declarations:true and noEmit:true should give you the result.

But are there cases where you want to ensure that declaration emit can succeed? For example, just something where you want to ensure that you won't get an error because of non-exported declarations?

I'm not sure I understand what you mean? Care to give an example?

The main usecase for only declarations is you have a pure js node project and you don't want to maintain hand typed .d.ts files. You don't care about emitting since your code is already in js.

@DanielRosenwasser I don't think this overrides --noEmitOnError, just --noEmit. I do think we might want to have a separate compiler option for this however, as always emitting declarations could have undesired consequences.

mhegazy · 2017-05-23T01:03:33Z

That was the easy part :) the interesting part is what do we do with JS-specific constructors.. e.g.

var x = require("..");   // should emit `import ... = require("..")`

function f() {
}
f.prototype.method = function() {}   // should probably emit a class?

etc..

moreover, today in the declaration emitter we do not walk expressions, but a lot of these JS constructs are expressions (assignments) and not declarations.. so how is that handled..

nojvek · 2017-05-24T23:23:13Z

@mhegazy I expect typescript to be esnext -> es old transpiler and typechecker. I would expect it to the same thing as if you renamed the .js file into .ts file and try to run declarations:true.

I think it's fair to say that it shouldn't magically understand prototype calls into classes or require calls into imports. If would expect allowJs + checkJs to treat js files with equal checks and integrity as a ts file but with types in jsdoc comments.

mhegazy · 2017-05-25T00:16:43Z

@mhegazy I expect typescript to be esnext -> es old transpiler and typechecker. I would expect it to the same thing as if you renamed the .js file into .ts file and try to run declarations:true.

I think it's fair to say that it shouldn't magically understand prototype calls into classes or require calls into imports. If would expect allowJs + checkJs to treat js files with equal checks and integrity as a ts file but with types in jsdoc comments.

TS already supports these patterns.. see https://github.com/Microsoft/TypeScript/wiki/Type-Checking-JavaScript-Files..
I am assuming the feature here is that, now that the compiler figured out my types, write it to a .d.ts file.

nojvek · 2017-05-25T00:17:10Z

@mhegazy are you also okay with declarations:true and noEmit:true being the flags to only emit .d.ts declarations?

mhegazy · 2017-05-25T00:18:38Z

@mhegazy are you also okay with declarations:true and noEmit:true being the flags to only emit .d.ts declarations?

This is the easy part as i said. the hard part is emitting the declaration file. we should figure out that first.

nojvek · 2017-05-25T00:21:51Z

I am assuming the feature here is that, now that the compiler figured out my types, write it to a .d.ts file.

This would mean the declaration emitter is smart about expressions right.

e.g.
var x = import('foo') => import * as x from 'foo'

Can this come in as a second PR? Do you consider not making those transformations a breaking change?

mhegazy · 2017-05-25T00:43:32Z

This would mean the declaration emitter is smart about expressions right.
e.g.
var x = import('foo') => import * as x from 'foo'

yes, and more importantlly:

/**
 * @typedef {Object} SpecialType - creates a new type named 'SpecialType'
 * @property {string} prop1 - a string property of SpecialType
 * @property {number} prop2 - a number property of SpecialType
 */
/** @type {SpecialType} */
var specialTypeObject;

would look like:

type SpecialType: {prop1: string, prop2: number };
declare var specialTypeObject: SpecialType;

mhegazy · 2017-05-25T00:45:11Z

Can this come in as a second PR? Do you consider not making those transformations a breaking change?

it is not being a breaking change or not. i am just questing the value of writing a .d.ts file that is different from what the compiler understands. to me, a .d.ts should fill in the place of a .js/.ts. so it should have the information you need to build against the file it describes.

nojvek · 2017-05-25T16:43:40Z

oh yes, I totally agree with that.

so here's the declaration transforms that I see you mention.

Class

function X()
X.prototype.foo = function(a, b){}

=>

declare class X {
  foo(a, b);
}

Interface

/**
 * @typedef {Object} SpecialType - creates a new type named 'SpecialType'
 * @property {string} prop1 - a string property of SpecialType
 * @property {number} prop2 - a number property of SpecialType
 */
/** @type {SpecialType} */
var specialTypeObject;

=>

type SpecialType: {
  prop1: string, 
  prop2: number 
};
declare var specialTypeObject: SpecialType;

Any others?

I may need someone to help me out, seems pretty involved 😮

mhegazy · 2017-05-25T19:08:03Z

The complete list of things we do there can be captured by searching the code base for isInJavaScriptFile. this conditions all the special behaviors related to a .js file.

We have a write-up for this in https://github.com/Microsoft/TypeScript/wiki/Type-Checking-JavaScript-Files. For JSDoc supported patterns, you want to look at https://github.com/Microsoft/TypeScript/wiki/JSDoc-support-in-JavaScript for more details.

I do not think you need to think about the whole set now. what is important is thinking of how do we add support for such things.
For JSDoc types, i would expect these to be serialize using functions like emitResolver.writeTypeOfDeclaration. thought that will not write a type alias declaration for @typedef tag, but it will serialize it as an anonymous type, and i would say this is a good start.

For the function -> class transformation, i would assume we have special code in writeFunctionDeclaration that notices that this is a JS file, with some function in it, and then use the symbol to write the class instead of the node.

rbuckton · 2017-05-30T17:21:46Z

src/compiler/declarationEmitter.ts

@@ -1888,7 +1888,7 @@ namespace ts {
    /* @internal */
    export function writeDeclarationFile(declarationFilePath: string, sourceFileOrBundle: SourceFile | Bundle, host: EmitHost, resolver: EmitResolver, emitterDiagnostics: DiagnosticCollection, emitOnlyDtsFiles: boolean) {
        const emitDeclarationResult = emitDeclarations(host, resolver, emitterDiagnostics, declarationFilePath, sourceFileOrBundle, emitOnlyDtsFiles);
-        const emitSkipped = emitDeclarationResult.reportedDeclarationError || host.isEmitBlocked(declarationFilePath) || host.getCompilerOptions().noEmit;
+        const emitSkipped = emitDeclarationResult.reportedDeclarationError || host.isEmitBlocked(declarationFilePath);


@mhegazy Do we want to emit declaration files for --declaration --noEmit for TypeScript (i.e. without --allowJs)? This would be a breaking change and could have undesired consequences. Do we need to have a new compiler option for this behavior?

@mhegazy Do we want to emit declaration files for --declaration --noEmit for TypeScript (i.e. without --allowJs)? This would be a breaking change and could have undesired consequences. Do we need to have a new compiler option for this behavior?

I agree, we should not do that. either we emit declarations for Js files with --declaration --allowjs, and users have to pass a --outDir tmp, then ignore it (this is still potentially a breaking change since we did not emit these files in the past so current --allowjs users would find this surprising), or we add a new flag --emitDeclarationsOnly.

Bikeshedding here: perhaps --noEmitJs as a looser --noEmit that still emits declarations but not js output?

that sounds better than my proposal.

nojvek · 2017-05-30T17:44:18Z

Happy to add another compiler option. May be something like noEmitJs:true.

…

On Tue, May 30, 2017 at 10:24 AM, Ron Buckton ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/compiler/program.ts <#15911 (comment)> : > @@ -904,7 +904,13 @@ namespace ts { let declarationDiagnostics: Diagnostic[] = []; if (options.noEmit) { - return { diagnostics: declarationDiagnostics, sourceMaps: undefined, emittedFiles: undefined, emitSkipped: true }; + // Allow only emitting of declarations + if (options.declaration) { @DanielRosenwasser <https://github.com/danielrosenwasser> I don't think this overrides --noEmitOnError, just --noEmit. I do think we might want to have a separate compiler option for this however, as always emitting declarations could have undesired consequences. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#15911 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-JVNmsbSs2Bq3uynjwH0VeOvdFdAiOks5r_FDBgaJpZM4NeOwo> .

nojvek · 2017-05-30T17:45:14Z

Although there seems to be already one as an /*@internal*/, I have no idea what that does in the compiler options object though.

…

On Tue, May 30, 2017 at 10:43 AM, Noj Vek ***@***.***> wrote: Happy to add another compiler option. May be something like noEmitJs:true. On Tue, May 30, 2017 at 10:24 AM, Ron Buckton ***@***.***> wrote: > ***@***.**** commented on this pull request. > ------------------------------ > > In src/compiler/program.ts > <#15911 (comment)> > : > > > @@ -904,7 +904,13 @@ namespace ts { > let declarationDiagnostics: Diagnostic[] = []; > > if (options.noEmit) { > - return { diagnostics: declarationDiagnostics, sourceMaps: undefined, emittedFiles: undefined, emitSkipped: true }; > + // Allow only emitting of declarations > + if (options.declaration) { > > @DanielRosenwasser <https://github.com/danielrosenwasser> I don't think > this overrides --noEmitOnError, just --noEmit. I do think we might want > to have a separate compiler option for this however, as always emitting > declarations could have undesired consequences. > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#15911 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AA-JVNmsbSs2Bq3uynjwH0VeOvdFdAiOks5r_FDBgaJpZM4NeOwo> > . >

nojvek · 2017-05-30T21:06:04Z

`noEmitJs` it is. I'll make it happen.

…

On Tue, May 30, 2017 at 2:01 PM, Mohamed Hegazy ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/compiler/declarationEmitter.ts <#15911 (comment)> : > @@ -1888,7 +1888,7 @@ namespace ts { /* @internal */ export function writeDeclarationFile(declarationFilePath: string, sourceFileOrBundle: SourceFile | Bundle, host: EmitHost, resolver: EmitResolver, emitterDiagnostics: DiagnosticCollection, emitOnlyDtsFiles: boolean) { const emitDeclarationResult = emitDeclarations(host, resolver, emitterDiagnostics, declarationFilePath, sourceFileOrBundle, emitOnlyDtsFiles); - const emitSkipped = emitDeclarationResult.reportedDeclarationError || host.isEmitBlocked(declarationFilePath) || host.getCompilerOptions().noEmit; + const emitSkipped = emitDeclarationResult.reportedDeclarationError || host.isEmitBlocked(declarationFilePath); that sounds better than my proposal. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#15911 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-JVPDmG0m6bXe2Bv0h2YCwdYOpNDDEks5r_IO1gaJpZM4NeOwo> .

…g declarations on its own.

asvetliakov · 2017-11-05T20:04:58Z

What's status of this? Since it's now possible to use only babel for TS compilation, this PR will be very helpful.

mhegazy · 2017-11-06T20:58:30Z

Closing for now since it has been stale for over 6 months now. As noted earlier in #15911 (comment) the challenge here is figuring out how to emit declarations for JS declarations that come from expressions.

nojvek · 2017-11-07T01:00:23Z

I'm gonna take another stab at this. Hopefully get more time over Vacation to work on this.

…

On Tue, Nov 7, 2017 at 12:59 AM Mohamed Hegazy ***@***.***> wrote: Closed #15911 <#15911>. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#15911 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-JVFnnIyUCCaHu7hoBovJo8-oTSRj6ks5sz3MngaJpZM4NeOwo> .

msftclas added the cla-already-signed label May 17, 2017

nojvek mentioned this pull request May 17, 2017

Allow --declaration with --allowJs #7546

Closed

DanielRosenwasser reviewed May 19, 2017

View reviewed changes

mihailik mentioned this pull request May 24, 2017

Use typescript's --checkJS to validate preact doc comments preactjs/preact#679

Closed

rbuckton reviewed May 30, 2017

View reviewed changes

Noj Vek added 12 commits June 3, 2017 15:06

Allow just generation of declarations for js files and just generatin…

9405068

…g declarations on its own.

typescript should warned me about this typo.

919dd34

allowJs cannot be specified with option 'declaration'

42877fb

update allowJs and declaration baselines in project tests

73aad51

Fix harness to allow noEmit + declaration

9d4b2b5

Merge with master

2ecf2be

getDiff is a function

0376177

too much renaming

ff1f073

do not find+replace in the wild

3c50a0f

diff in error

4b6ce80

noEmitJs

aa4d816

duplicate variable tests

0f25758

TEMP: faster testing

6795308

msftgits removed the cla-already-signed label Sep 26, 2017

mhegazy closed this Nov 6, 2017

nojvek mentioned this pull request Jan 29, 2018

allowJs:true + declaration:true #21455

Closed

9 tasks

microsoft locked and limited conversation to collaborators Jun 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow just generation of declarations, also allowJs #15911

Allow just generation of declarations, also allowJs #15911

nojvek commented May 17, 2017 •

edited

Loading

DanielRosenwasser May 19, 2017

nojvek May 19, 2017

DanielRosenwasser May 19, 2017 •

edited

Loading

nojvek May 19, 2017

rbuckton May 30, 2017

mhegazy commented May 23, 2017

nojvek commented May 24, 2017

mhegazy commented May 25, 2017

nojvek commented May 25, 2017

mhegazy commented May 25, 2017

nojvek commented May 25, 2017

mhegazy commented May 25, 2017

mhegazy commented May 25, 2017

nojvek commented May 25, 2017 •

edited

Loading

mhegazy commented May 25, 2017

rbuckton May 30, 2017

mhegazy May 30, 2017

rbuckton May 30, 2017

mhegazy May 30, 2017

nojvek commented May 30, 2017 via email

nojvek commented May 30, 2017 via email

nojvek commented May 30, 2017 via email

asvetliakov commented Nov 5, 2017

mhegazy commented Nov 6, 2017

nojvek commented Nov 7, 2017 via email

Allow just generation of declarations, also allowJs #15911

Allow just generation of declarations, also allowJs #15911

Conversation

nojvek commented May 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielRosenwasser May 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhegazy commented May 23, 2017

nojvek commented May 24, 2017

mhegazy commented May 25, 2017

nojvek commented May 25, 2017

mhegazy commented May 25, 2017

nojvek commented May 25, 2017

mhegazy commented May 25, 2017

mhegazy commented May 25, 2017

nojvek commented May 25, 2017 • edited Loading

mhegazy commented May 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nojvek commented May 30, 2017 via email

nojvek commented May 30, 2017 via email

nojvek commented May 30, 2017 via email

asvetliakov commented Nov 5, 2017

mhegazy commented Nov 6, 2017

nojvek commented Nov 7, 2017 via email

nojvek commented May 17, 2017 •

edited

Loading

DanielRosenwasser May 19, 2017 •

edited

Loading

nojvek commented May 25, 2017 •

edited

Loading