Skip to content

allowJs:true + declaration:true#21455

Closed
nojvek wants to merge 6 commits intomicrosoft:masterfrom
nojvek:emitJsDeclarations
Closed

allowJs:true + declaration:true#21455
nojvek wants to merge 6 commits intomicrosoft:masterfrom
nojvek:emitJsDeclarations

Conversation

@nojvek
Copy link
Copy Markdown
Contributor

@nojvek nojvek commented Jan 29, 2018 •
edited
Loading

  • There is an associated issue. Fixes Allow --declaration with --allowJs #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

** TODO **

From previous PR: #15911

  • Fix commented out tsserver test. Not sure what the correct fix is
  • ES5 prototype style function definitions
function f() {
}
f.prototype.method = function() {}   // should probably emit a class?
  • @typedef {Object} SpecialType -> Interfaces
/**
 * @typedef {object} SpecialType1 - creates a new type named 'SpecialType'
 * @property {string} prop1 - a string property of SpecialType
 * @property {number} prop2 - a number property of SpecialType
 * @property {number=} prop3 - an optional number property of SpecialType
 */
/** @type {SpecialType1} */
var specialTypeObject1;
  • Recognize @private/@readonly -> private X / readonly X
  /** @private */
  _render() {...}

  /** @readonly */
  static observedAttributes() {...}

This was referenced Jan 29, 2018
Closed
Closed
emolr added a commit to framejs/framejs that referenced this pull request Feb 8, 2018
@emolr
updated lit-html version and are now exporting h and html from their …
44922ad 
…modules to prevent version mismatch. furthermore lit-html and preact is imported from js, this will give an compile error see this microsoft/TypeScript#21455. but it compiles fine, and makes it possible to use the renderers without any module bundling from the users side
sheetalkamat
sheetalkamat reviewed Feb 12, 2018
View reviewed changes
Comment thread src/harness/unittests/tscWatchMode.ts
// More comment`;
const configFileContentAfterComment = `
"compilerOptions": {
"allowJs": true,
Copy link
Copy Markdown
Member

@sheetalkamat sheetalkamat Feb 12, 2018

Choose a reason for hiding this comment

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

you could change this to set { "inlineSourceMap": true, "sourceMap": true } to make sure the errors are reported on those lines in the config file.

@que-etc que-etc mentioned this pull request Feb 20, 2018
Closed
@dheeraja00 dheeraja00 mentioned this pull request Feb 26, 2018
Open
@GirlBossRush
Copy link
Copy Markdown

GirlBossRush commented Feb 28, 2018

This looks awesome! Is there an ETA to release?

@nojvek
Copy link
Copy Markdown
Contributor Author

nojvek commented Feb 28, 2018 via email

@DSchroer
Copy link
Copy Markdown

DSchroer commented Feb 28, 2018

I did a quick test of this on a project I am developing for. It works great except that it automatically detects require("SOMETHING") as a module import even if the TypeScript module setting is set to none.

This is a weird quirk of the code I am dealing with but it would be nice to be able to disable module detection. Since in my case require is just another function.

@nojvek
Copy link
Copy Markdown
Contributor Author

nojvek commented Mar 1, 2018

Yeah there are lots of little quirks like that to work out.

But being able to auto-generate types for js projects is going to be amazing.

@weswigham
Copy link
Copy Markdown
Member

weswigham commented Mar 1, 2018

@nojvek Once #21930 is merged, I'd like to write a transformer for JS files that simply iterates over their exported symbols, gets their declared types, and uses a modified typeToTypeNode on them to generate declarations for all required symbols. It needs to be a very different process from our usual subtractive declaration emit.

@nojvek
Copy link
Copy Markdown
Contributor Author

nojvek commented Mar 1, 2018

ooo I like it. Do you have a guesstimate for the merge ?

@gnomeontherun gnomeontherun mentioned this pull request Mar 7, 2018
Merged
@tdumitrescu tdumitrescu mentioned this pull request Jun 10, 2018
Merged
@nojvek
Copy link
Copy Markdown
Contributor Author

nojvek commented Jun 21, 2018

Typescript 2.9 has introduced some nice goodies. I am going to take another stab at this.

Really liking what the improvements to jsdoc and codecompletion. Whoever did that. Thank you!

@j-f1 j-f1 mentioned this pull request Jul 10, 2018
Closed
@ahocevar ahocevar mentioned this pull request Aug 10, 2018
Closed
3 tasks
@ahocevar
Copy link
Copy Markdown

ahocevar commented Aug 17, 2018

What needs to be done to finish this effort?

@nojvek
Copy link
Copy Markdown
Contributor Author

nojvek commented Aug 22, 2018 •
edited
Loading

I basically just need to re-focus and get back into it. TSC has moved some pieces around so need to re-base and figure out the merge conflicts + tests.

Hopefully we don't end up bikeshedding again. Been trying to land this since last year now 😞

@weswigham
Copy link
Copy Markdown
Member

weswigham commented Aug 22, 2018

You probably want to start fresh rather than attempt a rebase - the declaration emit pipeline has entirely changed since this PR was last updated.

@RyanCavanaugh
Copy link
Copy Markdown
Member

RyanCavanaugh commented Sep 5, 2018

@nojvek thanks for the work so far! I'm going to close this since it's going to require a substantial rebasing anyway. Looking forward to the next iteration.

@ahocevar ahocevar mentioned this pull request Oct 1, 2018
Closed
5 tasks
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sheetalkamat sheetalkamat sheetalkamat left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow --declaration with --allowJs

7 participants

@nojvek @GirlBossRush @DSchroer @weswigham @ahocevar @RyanCavanaugh @sheetalkamat