[api-extractor] Improve symbol analyzer to support "export * from" constructs

[octogonz]

This PR turned into a full rewrite of the symbol analyzer, adding the following capabilities:

• Support export * from './localFile'; by merging those declaration into the entry point as before

• Support export * from 'external-package' by emitting an export * in the output

• Support export { A } from './localFile'; where ./localFile.ts obtains A by doing export * from 'external-package';

• Allow a declaration to be exported more than once:

class X { }
export { X as A }
export { X as B}

[octogonz]

@natalieethell FYI #Resolved

[adventure-yunfei]

FYI: The review file generating will break with export from external package like following:

export { SemVer } from 'semver';

The error is: Error: Child declaration not found for the specified node.

Note that the test case api-extractor-test-02 also includes a similar declaration export { ReexportedClass as RenamedReexportedClass3 } from 'api-extractor-test-01';, but it succeeds, because the nominal is set as true as the check this._packageMetadataManager.isAedocSupportedFor passes and so other declarations are parsed.

And, in such case that this._packageMetadataManager.isAedocSupportedFor returns true, we'll get the similar issue that introduced by namespace (see related issue #1001). The real cause in such case is: if we import something from another package, but it's not the rootAstSymbol, it'll always fail since the later check (followAliasesResult.astImport && !astSymbol.imported) will always be true (the astSymbol itself has "astImport", but its rootAstSymbol doesn't).

I found this problem when trying to add support for export * as blah from './local-module';. Seems this related problem is really tough, cause of some special case handling... #Resolved

[adventure-yunfei]

And another one, also for the case:

export { SemVer } from 'semver';

The generated api.json would include detail type declarations for Semver (because we analyzed it as the check this._program.isSourceFileFromExternalLibrary returned false). But since we won't analyze it now, the new api.json will contain nothing except the name Semver... It was a wrong condition checking but seemed to produce the right results~~ #Resolved

[octogonz]

@adventure-yunfei I just pushed an update I was working on that hopefully should address this case.

BTW the main idea behind Error: Child declaration not found for the specified node is that nodes from an external package are treated as "nominal" (i.e. only partially analyzed) because we don't expect for any of the generators to process them. Then when that turns out not to be the case, the expected associated AstDeclaration is not found. These bugs should be eliminated once we have fixed all the cases where the symbol analyzer mistakenly treats external symbols as being part of the local project.

The algorithm in my PR branch is still missing a major case. It should now correctly handle these cases:

src/index.ts

export * from './SomeFile';
export * from 'other-package';

...if we have src/SomeFile.ts:

export class X { }

But it does not yet handle this case:

src/index.ts

export { X } from './OtherFile';

...if we have src/OtherFile.ts:

export * from 'other-package';

I don't believe there is an easy solution for that. Replacing ts.TypeChecker.getExportsOfModule() was not too difficult, because export * is a simple construct. But as far as I can tell, this second case requires replacing ts.getImmediateAliasedSymbol() which has to handle all the different import/export syntaxes. The underlying problem is that the compiler treats NPM module resolution as a simplistic mechanism for finding source files, and does not actually track NPM package boundaries in any way that can be discovered later by API Extractor.

I'm hoping to get some time over the holiday to finish this. Originally I was going to defer this work until after version 7 was released, but several teams at work are encountering these issues, so they can't use version 7 without this fix. #Resolved

[adventure-yunfei]

I've discovered some designs of api-extractor. To solving this kind of "export from external package" problem, my idea is to always analyze them no matter local or external, instead of blocking analyzing for external. api-extractor is treating external symbols too specially and I wonder it may lead to potential risks.

For *.d.ts rollup or review file generator, detail external declarations may not be helpful (as they simply output the original export ... from '...' statements). But for api.json generator, I think it may be.

Consider the case (for api.json generator):

export { ExternalModule } from 'external-package';

For api.json (or related api-documenter output), what it should be? Two choices:

1. Hide declaration details and just display an reexport statement from the external package
2. Hide reexport details and display the actual declaration of ExternalModule similar to any other local declaration.

From the api sight, I would prefer the second one.

And, for any other generators including those that may be added in future, I would prefer to give all details (declaration details, reexport informations) to the generator and let themselves to choose what the want to generate. e.g.:

• for every local symbol, it would contain declaration details (keep the same)
• for every external symbol (no matter it's created directly by an export statement or indirectly referenced), it would contain both declaration details and the external package info (e.g. what's the external package? Is this symbol exported?)

When the *.d.ts and review file generators see an external symbol, they generate a reexport statement; When the api.json generator (or any other generator taking case of declaration details instead of reexport info), it generate the declaration details.

Just my simple thoughts. I'm learning api-extractor ~ #Resolved

[octogonz]

@adventure-yunfei wrote:

For api.json (or related api-documenter output), what it should be? Two choices:

1. Hide declaration details and just display an reexport statement from the external package
2. Hide reexport details and display the actual declaration of ExternalModule similar to any other local declaration.

From the api sight, I would prefer the second one.

I'm not sure I agree with this. We are encountering this situation in real world scenarios, e.g. office-ui-fabric-react has a number of instances as part of a transitional step to split up a monolithic NPM package into smaller packages.

In my view, this sort of thing:

export { ExternalModule } from 'external-package';

...is its own kind of API item that is similar to a TypeScript type alias. The API reference web site will already have a full set documentation for ExternalModule and all of its members in the section for external-package. It would seem counterproductive to duplicate all of this content in the section for this-package. Besides being confusing for the human audience, and it might negatively impact other systems such as search engine optimization (SEO).

For the *.d.ts rollup, it would be technically incorrect to duplicate the declarations. (For example, the TypeScript compiler will not consider class types to be interchangeable if they contain any private members.)

The *.api.ts review file is a somewhat special scenario. Since this is used to detect compatibility breaks, it may be useful to inline the full signature of the external declaration. Issue #896 is essentially asking for this. In the comments for that issue I gave some reasons why it should perhaps be an "opt-in" behavior.

In any case, it seems clear that API Extractor's analyzer needs to know which package a declaration comes from, in order to solve these sorts of problems. #Resolved

[octogonz]

the main idea behind Error: Child declaration not found for the specified node is that nodes from an external package are treated as "nominal" (i.e. only partially analyzed) because we don't expect for any of the generators to process them.

Hmm... Here and here I came across some usages of the term "nominal" to mean "type compatibility based on the name of a type, not just its signature". I wonder if API Extractor should choose a different name for our concept. @nickpape-msft @iclanton #Resolved

[adventure-yunfei]

In my view, this sort of thing:

export { ExternalModule } from 'external-package';

...is its own kind of API item that is similar to a TypeScript type alias. The API reference web site will already have a full set documentation for ExternalModule and all of its members in the section for external-package. It would seem counterproductive to duplicate all of this content in the section for this-package. Besides being confusing for the human audience, and it might negatively impact other systems such as search engine optimization (SEO).

Agree with you now. Reexported items from external packages should be handled in user side, not the api-extractor core. It's a special case to merge some packages' contents.

#Resolved

[iclanton]

Can you split the compiler internals into a separate file so it's clear that's what they are. It'll also be easier to find problems if the compiler internals change. #Resolved

[octogonz]

Fixed

---

In reply to: 248125894 [](ancestors = 248125894)

[iclanton]

nameForEmit [](start = 81, length = 11)

Sanetize? #WontFix

[iclanton]

emitWithExportKeyword [](start = 13, length = 21)

Maybe shouldInlineExport #Resolved

[octogonz]

Good suggestion

---

In reply to: 248127101 [](ancestors = 248127101)

[iclanton]

[octogonz]

This is fixed now.

---

In reply to: 449750119 [](ancestors = 449750119)

[octogonz]

I renamed it to "nominalAnalysis"

---

In reply to: 451778545 [](ancestors = 451778545)

[octogonz]

This PR was published as the beta release api-extractor@7.0.10 and api-documenter@7.0.15.

To try it, do npm install @microsoft/api-extractor@beta