rushstack
rushstack copied to clipboard
[api-extractor] Improve API Extractor's ability to handle `export =` patterns
Summary
This PR improves API Extractor's ability to handle export =
patterns. Ideally I would say "this PR allows API Extractor to support all kinds of export =
patterns", but I don't have an exhaustive understanding of all of them, so I don't have quite enough confidence to say that.
This issue was originally discovered in https://github.com/microsoft/rushstack/issues/2220.
Details
API Extractor crashes today with "Unable to analyze the export..." when it encounters the following export =
patterns:
Pattern 1
// file1.ts
export = SomeClass;
class SomeClass {}
// file2.ts
import SomeClass from './file1';
// (requires `esModuleInterop: true` in tsconfig)
Pattern 2
// file1.ts
export = ns;
namespace ns {
export class SomeClass {}
}
// file2.ts
import { SomeClass } from './file1';
These two patterns are now supported. The root cause was that API Extractor's ExportAnalyzer.tryGetExportOfAstModule
method previously was unable to resolve the AstEntity
for these kinds of exports. Now, its resolution logic has been extended to follow these patterns. Also, both patterns are now tested in the api-extractor-scenarios "exportEquals2" test.
It's possible that the API Extractor team doesn't want to support these patterns, from some of the discussion a few years ago on https://github.com/microsoft/rushstack/issues/2220. If that's the case, that's fine, feel free to reject this PR.
How it was tested
Added a new api-extractor-scenarios test "exportEquals2" that succeeds with valid output with this PR, and fails with an API Extractor InternalError
without this PR.
Also ran rush rebuild
across the entire repo and confirmed that everything else looked the same.
Impacted documentation
No impacted documentation.
Wow! 👏😄
It's possible that the API Extractor team doesn't want to support these patterns, from some of the discussion a few years ago on #2220. If that's the case, that's fine, feel free to reject this PR.
I wasn't enthusiastic about spending time on it, but of course we'll happily accept a PR if the implementation is straightforward. Thanks!
Great! Before merge, can you test this on a few other monorepos? I tested it on my company's monorepo and it seems to work fine. Alternatively, if you send me a link to others you know of, I'm happy to test myself. I'm not 100% confident it doesn't introduce a regression of some kind. 😅