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

Comment regarding API Extractor #13

Closed
octogonz opened this issue Mar 7, 2019 · 7 comments
Closed

Comment regarding API Extractor #13

octogonz opened this issue Mar 7, 2019 · 7 comments

Comments

@octogonz
Copy link

octogonz commented Mar 7, 2019

@Swatinem I noticed this text in the README:

  • API Extractor
    an official Microsoft project, which however is super complicated and I was not
    able to get it to work.

As someone who works on API Extractor, I'm curious what problems you encountered, and whether there's anything we could do to address them. API Extractor 7 has been in "beta" for the past few months because the analyzer was overhauled. We're now very close to releasing it. Since it's a new major version, this is a pretty good time to make breaking changes to the command line or config files if something should be improved/simplified.

@Swatinem
Copy link
Owner

Swatinem commented Mar 9, 2019

Hi @octogonz and thanks for reaching out.
I already commented on my opinion here: timocov/dts-bundle-generator#68 (comment)
In short, it comes down to configuration and having to maintain intermediate artifacts.

What I notice right away when browsing the repo is this:
https://github.com/Microsoft/web-build-tools/blob/93b87bb33edea79563401c1f4e3e230919917da2/apps/api-extractor/package.json#L43

Why? I use 3.3.3333 and don’t want to install two versions in parallel (TS is damn 40+ MB!!! microsoft/TypeScript#27891) That thing should be a peerDependency, no?

But I will give AE another shot once I find some time to play around with it and report back here :-)

@octogonz
Copy link
Author

octogonz commented Mar 13, 2019

Why? I use 3.3.3333 and don’t want to install two versions in parallel (TS is damn 40+ MB!!! Microsoft/TypeScript#27891) That thing should be a peerDependency, no?

I agree that intuitively, you should just be able to drop in whatever compiler you like as a peer dependency.

However, API Extractor doesn't just run tsc; it invokes the compiler engine as a library to analyze source files. The abstract syntax tree (AST) data structure is very complex and constantly changes between compiler releases, and our usage is fairly elaborate. For example, API Extractor's .d.ts rollup feature is doing something that the compiler was not designed to do, so we have to use undocumented APIs from the compiler engine to fish around in the the internal structures to find the missing information. (To see a good example of this, search for TypeScriptInternals in ExportAnalyzer.ts). When upgrading the compiler version, there aren't any guarantees about the stability of these APIs.

For this reason, API Extractor requires a specific compiler version that has been tested and is known to work. The aim is for it to be the most recent stable version, and we provide a specific facility so that you can use the latest API Extractor / compiler to analyze projects that are built using an older compiler version.

But it's somewhat costly to stay on the absolute bleeding edge. For example, there is already a PR microsoft/rushstack#1124 upgrading to 3.3.3333 but it's stuck because there were some compatibility issues.

@Swatinem
Copy link
Owner

@octogonz sorry for the long delay. I finally came around trying api-extractor@7.1.0 after quite a long time.

First off, very good job on the configuration and the docs. I got started immediately with a < 10 loc config file! 👍

I do have to emit temporary build artifacts from tsc, but I may reconsider my opinion on that.
At work, we have some >3k ts files (including external dependencies), and both typechecking and building are getting painfully slow so I might consider using typescript project references and --incremental to improve that situation.

I have also tried a few of the testcases that rollup-plugin-dts does not yet handle well, and I am surprised that api-extractor is struggling with the same!

So after all, I am quite surprised that my plugin handles a few of these edge-cases a bit better than api-extractor.

Anyway, thanks for reaching out, and I will update my README slightly to point to these discussions

@Swatinem
Copy link
Owner

Heads up:
I updated my Readme, pointing to this issue and other discussions: https://github.com/Swatinem/rollup-plugin-dts#alternatives feel free to voice any suggestions.

@octogonz
Copy link
Author

I updated my Readme, pointing to this issue and other discussions: https://github.com/Swatinem/rollup-plugin-dts#alternatives feel free to voice any suggestions.

Cool, thanks! BTW it looks like the API Extractor line has broken link. It should be api-extractor.com.

So after all, I am quite surprised that my plugin handles a few of these edge-cases a bit better than api-extractor.

Does your tool handle export * from? We recently added support for that because lots of people were asking for it. It's tricky because you can have an entry point that does this:

package1/src/index.ts

export class A { }
export { B } from './file1';

and then you have this:

package1/src/file1.ts

export * from 'package2';
export * from 'package3';

Formally, the determination of whether B comes from package2 or package3 should be deferred until runtime. Whereas the rollup has to make this determination at build time.

Also, ES6 allows cycles such as two packages doing export * from each other, which requires the graph to be traversed very carefully.

@Swatinem
Copy link
Owner

Fixed the link, and added new testcases:
83dc865

Yes, export * from is supported through rollup. Also circular dependencies (without export *) are supported as well. Rollup itself issues a warning for that case though.

Do you have a specific testcase for circular dependencies involving export * that I can copy?

Also, I do have ideas how to solve the inline namespace import from external (interface Foo { bar: import("external") }) case, but it would involve some string manipulation of the code, which I would like to avoid as much as possible.

@Swatinem
Copy link
Owner

Looks like an issue in rollup itself, which fails your testcase even without my plugin.
Filed a bug here: rollup/rollup#2815

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

No branches or pull requests

2 participants