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

factory.createSourceFile doesn't accept JSDoc node (typings issue) #44151

Closed
bali182 opened this issue May 18, 2021 · 8 comments
Closed

factory.createSourceFile doesn't accept JSDoc node (typings issue) #44151

bali182 opened this issue May 18, 2021 · 8 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@bali182
Copy link

bali182 commented May 18, 2021

Bug Report

πŸ”Ž Search Terms

  • factory
  • createSourceFile
  • typings

πŸ•— Version & Regression Information

In v4.2.4 (current latest)

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

import { factory as f, SyntaxKind, NodeFlags } from 'typescript'

const comment = f.createJSDocComment('test', [])
const file = f.createSourceFile(
    [comment], // Error here as createSourceFile needs a Statement array.
    f.createToken(SyntaxKind.EndOfFileToken), 
    NodeFlags.None
)

πŸ™ Actual behavior

Compiler error in factory#createSourceFile when a JSDoc[] node supplied as Statement[] is expected. If it's casted to any, its fine at runtime, so I assume it's just an issue with typings.

πŸ™‚ Expected behavior

Accept JSDoc as a root level element for factory#createSourceFile

@bali182 bali182 changed the title factory.createSourceFile doesn't accept JSDoc node factory.createSourceFile doesn't accept JSDoc node (typings issue) May 18, 2021
@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label May 27, 2021
@RyanCavanaugh
Copy link
Member

This might happen to work today, but it's not the API contract. You need to attach the JS Doc comment to an EmptyStatement or such.

@bali182
Copy link
Author

bali182 commented May 29, 2021

Awesome didn't know that, thanks!

@bali182 bali182 closed this as completed May 29, 2021
@bali182 bali182 reopened this Jun 21, 2021
@bali182
Copy link
Author

bali182 commented Jun 21, 2021

I started experimenting with this again, and it seems like there's no way to attach JSDocComments to other nodes. How do I do this? There seem to be no specific API method for this and setSyntheticLeadingComments doesn't accept JSDoc nodes. When parsing it seems to work beautifully, but how can I build this AST? There seems to be no jsDoc field on the nodes.

https://astexplorer.net/#/gist/63752d280452826f4031882c9943cd1a/dd91c7dbc66f70466e50f72a13c9a322fbd2d220

@RyanCavanaugh RyanCavanaugh added Question An issue which isn't directly actionable in code and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels Jun 21, 2021
@RyanCavanaugh
Copy link
Member

@rbuckton any advice?

@rbuckton
Copy link
Member

The JSDoc is kind of a weird byproduct of how we support JSDoc for type information. In general, JSDoc nodes aren't built to be used with transforms, except in a few specific cases in some of our refactors. We don't emit JSDoc nodes attached to a statement or any other node. Instead we emit either the original comments from a source file, or synthetic leading and trailing comments attached to a node. If we were to emit both, we would somehow need to differentiate between parsed JSDoc comments and unparsed non-jsdoc comments on any node so that we don't emit them twice.

The only way you can have a transformer attach a "jsdoc" comment would be to use addSyntheticLeadingComment (or one of the other related "synthetic comment" functions), similar to what we do here:

setSyntheticLeadingComments(result, [{
text: `*
${newComment.split("\n").map(c => ` * ${c}`).join("\n")}
`,
kind: SyntaxKind.MultiLineCommentTrivia,
pos: -1,
end: -1,
hasTrailingNewLine: true,
hasLeadingNewline: true,
}]);

One option is to use the createPrinter API to print out a formatted JSDoc comment, then strip off the leading /* and trailing */:

const printer = ts.createPrinter();
const output = printer.printNode(jsDoc).trim().replace(/^\/\*|\*\/$/, "");
addSyntheticLeadingComment(node, SyntaxKind.MultiLineCommentTrivia, output, /*hasTrailingNewLine*/ true);

This will have obvious caveats, since it would be difficult to make indentation match.

I could possibly see us modifying the synthetic comment functions to accept a JSDoc node, since we could differentiate on kind.

@bali182
Copy link
Author

bali182 commented Jun 22, 2021

Great explanation, thank you! The printer API trick seems like a good solution, as I format the output anyways with prettier, so should be ok to have indentation issues.

@bali182 bali182 closed this as completed Jun 22, 2021
@bali182
Copy link
Author

bali182 commented Jun 22, 2021

But accepting JSDoc as leading/trailing comments would be amazing for codegen!

@Jack-Works
Copy link
Contributor

I'm building an AST tree with factory API and printing it, there is no transformer involved. Is there any way I can attach comments on a synthesized node?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

4 participants