[api-extractor] crash when used with TS 4.7.2 moduleResolution Node16

[EisenbergEffect]

Summary

It seems that adding moduleResolution with a setting of Node16 to the tsconfig.json file causes @microsoft/api-extractor to fail every time. The error is as follows:

ERROR: Internal Error: getResolvedModule() could not resolve module name "./foo/bar.js"
/Users/johndoe/Documents/code/myrepo/my-package/dist/dts/index.d.ts:1:1
You have encountered a software defect. Please consider reporting the issue to the maintainers of this application.
error Command failed with exit code 1.

This is blocking us from shipping our major new library versions.

Repro steps

I've got a branch that demonstrates the issue here:

https://github.com/microsoft/fast/tree/users/eisenbergeffect/fast-element-utilities

Simply pull that down and run yarn in the root. You will see an error similar to the above.

Details

I've tried many combinations of settings, but the only thing that seems to make the error go away is not using the Node16 setting. Unfortunately, this is required for some of our packages since they leverage package.json exports. You can switch the fast-element package to Node to get it working but then you will observe the problem in fast-foundation (which requires Node16 because it leverages package exports from fast-element).

I have discussed this briefly with a member of the TS team and they suspect that api-extractor may be using some undocumented APIs, specifically getResolvedModule.

Standard questions

Question	Answer
@microsoft/api-extractor version?	7.24.1
Operating system?	Mac
API Extractor scenario?	rollups (.d.ts)
Would you consider contributing a PR?	Yes
TypeScript compiler version?	4.7.2
Node.js version (node -v)?	16.14.0

[DanielRosenwasser]

I did some work, and I believe the root issue is that getResolvedModule and its callers need to be updated since it now expects a third parameter. TypeScript's node16/nodenext modes now depend on the SourceFile and import kind (i.e. whether you have an ES import vs a require) to resolve accurately.

rushstack/apps/api-extractor/src/analyzer/TypeScriptInternals.ts

Line 84 in 7ffca33

public static getResolvedModule(

The fix should be

1. to add a third parameter to TypeScriptInternals.getResolvedModule and pass it along to TypeScript's internal function
2. expose (another) internal method on TypeScriptInternals called getModeForUsageLocation
3. update all callers to add a third argument long the lines of

TypeScriptInternals.getModeForUsageLocation(
  importOrExportDeclaration.getSourceFile(), importOrExportDeclaration)
)

[chrisdholt]

I did some work, and I believe the root issue is that getResolvedModule and its callers need to be updated since it now expects a third parameter. TypeScript's node16/nodenext modes now depend on the SourceFile and import kind (i.e. whether you have an ES import vs a require) to resolve accurately.

rushstack/apps/api-extractor/src/analyzer/TypeScriptInternals.ts

Line 84 in 7ffca33

public static getResolvedModule(

The fix should be

1. to add a third parameter to TypeScriptInternals.getResolvedModule and pass it along to TypeScript's internal function
2. expose (another) internal method on TypeScriptInternals called getModeForUsageLocation
3. update all callers to add a third argument long the lines of

TypeScriptInternals.getModeForUsageLocation(
  importOrExportDeclaration.getSourceFile(), importOrExportDeclaration)
)

In the midst of picking this up to try and unblock our coming release, but I'm hitting a bit of a wall trying to figure out the second argument.

/**
   * Gets the mode required for module resolution required with the addition of Node16/nodenext
   */
  public static getModeForUsageLocation(file: {impliedNodeFormat?: ts.SourceFile["impliedNodeFormat"]}, usage: ts.StringLiteralLike) {
    // Compiler internal:
    // https://github.com/microsoft/TypeScript/blob/v4.7.2/src/compiler/program.ts#L568

    return (ts as any).getModeForUsageLocation(file, usage);
  }

The above typing matches what Typescript expects, but unfortunately the API Extractor type signature for methods calling this don't seem to overlap in a way that's clear to me. It appears from the TS usage that moduleSpecifier should work here but I'm hitting an issue with string not being able to overlap with ts.StringLiteralLike. Any ideas @DanielRosenwasser or perhaps @iclanton if you have thoughts? If the fix is as straight forward as above and I can resolve the typing issue I can push up the work and a quick patch would help us unblock for our release!

rushstack/apps/api-extractor/src/analyzer/ExportAnalyzer.ts

Line 259 in d3ab07e

importOrExportDeclaration: ts.ImportDeclaration | ts.ExportDeclaration | ts.ImportTypeNode,

[DanielRosenwasser]

Why can't you just use ts.SourceFile as the parameter type for file?

[chrisdholt]

Why can't you just use ts.SourceFile as the second argument?

I'll give it a shot and see if that types accurately - I'm just not familiar enough with all the internal usage both in TS and API Extractor. Thanks for the quick response.

[chrisdholt]

Why can't you just use ts.SourceFile as the parameter type for file?

So given this, I think it looks like below, which doesn't seem to error.

      TypeScriptInternals.getModeForUsageLocation(
        importOrExportDeclaration.getSourceFile(), importOrExportDeclaration.getSourceFile()
      )

[DanielRosenwasser]

Gotcha, sorry, I didn't realize that the signature was declared like that internally (weird, but I assume necessary). You can copy the one from TS.

I also reread your last comment. I might have made a mistake last night. Using StringLiteralLike is likely right, but you might need to write the following instead

TypeScriptInternals.getModeForUsageLocation(importOrExportDeclaration.getSourceFile(), importOrExportDeclaration.moduleSpecifier);

[DanielRosenwasser]

I just realized, because the new function might not be available in older versions, the TypeScriptInternals one should gracefully return undefined if it's not available.