rushstack icon indicating copy to clipboard operation
rushstack copied to clipboard

[api-extractor] Improve API Extractor's ability to handle `export =` patterns

Open zelliott opened this issue 2 years ago • 3 comments

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.

zelliott avatar Jan 30 '23 22:01 zelliott

Wow! 👏😄

octogonz avatar Feb 04 '23 05:02 octogonz

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!

octogonz avatar Feb 04 '23 05:02 octogonz

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. 😅

zelliott avatar Feb 05 '23 15:02 zelliott