Skip to content
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

[WIP] Make allowJs and declaration options work together #27478

Closed
wants to merge 2 commits into from

Conversation

ahocevar
Copy link

@ahocevar ahocevar commented Oct 1, 2018

Not ready for review yet!

This is work in progress, and a follow-up on recently closed #21455. I'm pretty stuck already in early stages of this pull request, so if anyone is able and willing to help I'll be glad to give access to my branch and coordinate the effort.

I think the best way to address this is to add helper utilities to TypeScript that convert nodes from *.js files with JSDoc types to nodes that look exactly as if the types were defined in *.ts code. Any guidance or ideas about different approaches would be much appreciated.

  • There is an associated issue that is labeled
    'Bug' or 'help wanted' or is in the Community milestone
  • 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

Fixes #7546

@msftclas
Copy link

msftclas commented Oct 1, 2018

CLA assistant check
All CLA requirements met.

@ahocevar ahocevar changed the title [WIP] Allowjs declaration [WIP] Make allowJs and declaration options work together Oct 1, 2018
@ajafff
Copy link
Contributor

ajafff commented Oct 1, 2018

Does/will this enable project references with JS files (composite and allowJs)?

@ahocevar
Copy link
Author

ahocevar commented Oct 2, 2018

@ajafff No, the scope of this effort is limited to making declaration and allowJs work together.

@weswigham
Copy link
Member

weswigham commented Oct 2, 2018

So... this probably isn't what you wanna hear; but you probably want to use a completely separate transformer for JS files, when compared to the TS one. Because of how JS declarations are merged/inferred in a piecewise fashion, what you probably want to do is something along the lines of iterating over all the exported symbols (rather than the declarations in the file) and attempting to rebuild them from typechecker information, rather than their (potentially non-.d.ts-compatible) original declaration sites. For example, we have an upcoming PR to recognize Object.defineProperty calls in JS files as property declarations - in a .d.ts they would mean nothing (and are expressions, which we generally elide in declarations), yet that information must still be transformed and preserved somehow. (That's in addition to all the special assignments we already recognize.) And while we do have typeToTypeNode already written within the checker (which makes, say, translating a jsdoc'd const into a type annotated const easy (we have a quickfix that does this)), we have not yet written anything like interfaceToInterfaceNode or classToClassNode (I think), so this is probably quite a bit of work. Also things like jsdoc enums I think actually don't even have a precise way to represent them in a .d.ts, I think (although a type plus a namespace probably comes close).

@ahocevar
Copy link
Author

ahocevar commented Oct 5, 2018

Thanks for the explanations @weswigham. This sounds like a ton of work indeed, requiring more time than I'm able to spend on it. I'm closing this pull request, but won't delete my branch in case anyone else wants to pick it up from here.

@ahocevar ahocevar closed this Oct 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow --declaration with --allowJs
4 participants