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

[api-extractor] Rename .api.ts to .api.md to prevent TypeScript tools from processing it #1123

Closed
jakob101 opened this issue Mar 1, 2019 · 16 comments
Labels
effort: easy Probably a quick fix. Want to contribute? :-) enhancement The issue is asking for a new feature or design change priority The maintainers consider it to be an important issue. We should try to fix it soon.

Comments

@jakob101
Copy link
Member

jakob101 commented Mar 1, 2019

Hi,

Love the API extractor. Looks like a very powerful tool.
It would be great if we could add a big comment on top of the file "this file is generated, don't change it manually". Others might want to add "/* tslint:disable */" to it as well.

How do we get around doing this in our api.ts review files?

Thanks and great work!
Jakob

@octogonz
Copy link
Collaborator

octogonz commented Mar 1, 2019

Sounds related to this discussion: #1114

There we were talking about whether the .api.ts file extension is good or not. Historically nobody has complained about it, but if it's confusing to people/tools, perhaps we should reconsider changing it.

On the other hand, if you think a couple code comments would clear things up, that seems like a reasonable solution, too.

@iclanton
Copy link
Member

iclanton commented Mar 5, 2019

I think there should be a "This is a generated file" comment at the top of the files. It's a pretty standard practice for generated files.

@jakob101
Copy link
Member Author

jakob101 commented Mar 5, 2019

Sounds like a reasonable feature.

@octogonz octogonz added enhancement The issue is asking for a new feature or design change help wanted If you're looking to contribute, this issue is a good place to start! effort: easy Probably a quick fix. Want to contribute? :-) labels Mar 5, 2019
@vdanchenkov
Copy link

We had to add to tslint.json

  "linterOptions": {
    "exclude": ["**/etc/*.api.ts", "**/temp/*.api.ts"]
  }

And to root .prettierignore

**/etc/*.api.ts
**/temp/*.api.ts

Apparently, there is no way to ignore the whole file in prettier by the comment.

Not a big problem, but the different extension for API files would simplify setup for new users.

@octogonz
Copy link
Collaborator

octogonz commented Mar 9, 2019

Anyone have objections to changing the file extension to .tsapi?

@iclanton

@octogonz octogonz removed the help wanted If you're looking to contribute, this issue is a good place to start! label Mar 10, 2019
@octogonz octogonz changed the title [api-extractor] Generated review file cannot be edited [api-extractor] Rename .api.ts to .tsapi to prevent TypeScript tools from processing it Mar 10, 2019
@octogonz octogonz added the priority The maintainers consider it to be an important issue. We should try to fix it soon. label Mar 10, 2019
@octogonz
Copy link
Collaborator

octogonz commented Mar 11, 2019

BTW I chatted some with @iclanton about this.

We had some reservations about introducing a new file extension that is completely proprietary to API Extractor and has a fairly vague spec about the format of its content.

In VS Code, I was able to restore syntax highlighting by adding this my settings.json:

    "files.associations": {
      "*.tsapi": "typescript"
    }

But that causes VS Code to perform semantic analysis and add a ton of red underlines everywhere.

We discussed two alternative approaches:

  1. Propose a more general file extension that means "this file should be syntax-highlighted like TypeScript, but it's not real code, so don't try to do semantic analysis of it." Beyond API Extractor, this file extension could be useful for stuff like code samples or template files. The extension could be a pattern like *.ts.snippet, which would provide an easy way to support other languages (*.js.snippet, *.java.snippet, etc.) without generating tons of new extensions. And then API Extractor's specific file might get a triple extension library.api.ts.snippet heheh.

  2. Save the API reports using .md file extension (e.g. library.api.md), and wrap their contents with a ```ts code fence. This has the advantage of working with existing tooling: A GitHub diff will get nicely syntax highlighted as TypeScript, but no tool will try to compile it or analyze it. It does have the downside that Markdown editors might discover the files and try to normalize them; however, they would see a mostly empty file containing a big code fence, and it seems unlikely that normalization rules would try to modify a block of "code". Another issue is that Markdown's grammar can be fairly troublesome for a machine-generated output file. (For example, if our API report needed to show three backticks on a blank line for some reason, they would get misinterpreted as the code fence delimiter, and there's no escape character to get around this; it's simply impossible to express in Markdown.)

@octogonz
Copy link
Collaborator

@daspek do you have an opinion on this one? :D Ian thought 1 is a more ideal solution, but 2 is simpler. A third opinion would be helpful.

@octogonz
Copy link
Collaborator

After a bunch of debate, the winning vote was for library.api.md.

@octogonz octogonz changed the title [api-extractor] Rename .api.ts to .tsapi to prevent TypeScript tools from processing it [api-extractor] Rename .api.ts to .api.md to prevent TypeScript tools from processing it Mar 14, 2019
@octogonz
Copy link
Collaborator

Implemented in PR #1150

@qm3ster
Copy link

qm3ster commented Mar 14, 2019

Prettier definitely attacks ```ts blocks.

@octogonz
Copy link
Collaborator

Prettier definitely attacks ```ts blocks.

Is it easy to configure it to ignore the .api.md file extension?

@qm3ster
Copy link

qm3ster commented Mar 14, 2019

Eeeh... probably easy enough for anyone using api extractor.
You have to put *.api.md into a .prettierignore file.
Or, if they are insane, they can do

{
  "overrides": [
    {
      "files": "*.api.md",
      "options": {
        "requirePragma": true
      }
    }
  ]
}

which would mean the file won't get formatted as it doesn't have a <!-- @prettier --> or <!-- @format --> comment at the top.

But, alternatively, you could put <!-- prettier-ignore --> as a line before the ```ts in the generated files. This way it would format the file, but ignore the next statement, which is the whole code block.

Or <!-- prettier-ignore-start --> at the top and <!-- prettier-ignore-end --> at the bottom, then it won't try to attack anything else you may want to put in the file.

Oops, totally didn't see @vdanchenkov's comment, sorry.

@octogonz
Copy link
Collaborator

octogonz commented Mar 14, 2019

I feel like configuring Prettier to ignore *.api.md is a better solution than putting <!-- prettier-ignore-start --> / <!-- prettier-ignore-end --> directives in all the review files, since as more tools come on the scene we'd have to keep polluting our files with more and more directives. Whereas it's highly unlikely that any other tool will use the *.api.md file extension, so ignoring it is an easy one-line change for .prettierignore.

@qm3ster
Copy link

qm3ster commented Mar 15, 2019

Totally my opinion as well, should just be a note in the docs, as in many cases people just don't have a .prettierignore in the repo at all.

@octogonz
Copy link
Collaborator

Good point. I'll make a note to include that in the docs. The web site content will continue to refer to AE6 until we finally remove the "beta" tag for AE7. We're so close... just a few more PRs heheh.

@octogonz
Copy link
Collaborator

This was published as API Extractor beta release 7.0.27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: easy Probably a quick fix. Want to contribute? :-) enhancement The issue is asking for a new feature or design change priority The maintainers consider it to be an important issue. We should try to fix it soon.
Projects
None yet
Development

No branches or pull requests

5 participants