[api-extractor] API Extractor now properly runs on projects with tsconfig path mappings.

[zelliott]

Fixes #3291

Background

API Extractor uses the _isExternalModulePath method in ExportAnalyzer.ts to determine whether or not a particular module specifier (i.e. from an import or export statement) is part of the current package or an external package. A module specifier is determined to be part of the current package IFF (1) it is a relative path (i.e. ts.isExternalModuleNameRelative returns true) or (2) it is part of a bundled package.

However, the logic does not take into consideration any path mappings that may have been set up in the project's tsconfig. For example, consider the following project scenario:

• Project has no bundled packages.
• Project tsconfig has paths { "foo": ["some/path/to/foo"] } and baseUrl as ".".
• We're processing the statement import Foo from 'foo';

In this case, given ts.isExternalModuleNameRelative returns false for 'foo' and 'foo' isn't a bundled package, API Extractor considers this module specifier to be external and will not generate any API doc model nodes for it. However, the actual import after resolving any path mappings is './some/path/to/foo', which is relative. If the import was originally written as import Foo from './some/path/to/foo', we would have generated API doc model nodes for it. We should fix this inconsistency in behavior.

Solution

Update

Please see #3321 (comment) for latest approach.

Previous approach

This PR adds additional logic to the _isExternalModulePath method that - in addition to the criteria above - determines a module specifier is relative if it matches any tsconfig path mapping. We consider it relative in this case because the fully-resolved path is guaranteed to be relative, as the mappings are always resolved relative to "baseUrl". The logic handles two types of path mappings (the only two that I know exist):

• Simple path-like strings: some/path/to/import
• Paths with wildcards at the end: some/path/to/*

If there are other kinds of syntaxes that are allowed in path mappings, this PR may not handle them correctly. I couldn't find an exhaustive reference for the allowed syntax in path mappings.

Testing

I tested this change by adding a new test scenario within api-extractor-scenarios called pathMappings. This test scenario references path mappings set up in the project tsconfig. I then ran rush rebuild to validate that the correct .api.json was generated. Other test scenarios' .api.json files did not change.

Alternatives considered

I think there may be an argument to be made that ts.isExternalModuleNameRelative should take into consideration path mappings and return true for any path mapping matches. I'm curious what the API Extractor maintainers think of this idea. If so, then the fix for this bug would be instead within the TypeScript project, and we wouldn't have to amend the _isExternalModulePath method. Regardless, it may still make sense to check in this PR in the meantime if we think the discussion & change to ts.isExternalModuleNameRelative might take time.

[zelliott]

@octogonz I remember you saying a while back that path mappings are strongly deprecated/discouraged within the Rush Stack world. Can you remind me why that's the case?

[octogonz]

Alternatives considered

I think there may be an argument to be made that ts.isExternalModuleNameRelative should take into consideration path mappings and return true for any path mapping matches. I'm curious what the API Extractor maintainers think of this idea. If so, then the fix for this bug would be instead within the TypeScript project, and we wouldn't have to amend the _isExternalModulePath method.

@rbuckton @DanielRosenwasser Do you have any opinions about this TypeScript API question? Changing the semantics of ts.isExternalModuleNameRelative might be risky, but perhaps a second API could be introduced?

[octogonz]

@octogonz I remember you saying a while back that path mappings are strongly deprecated/discouraged within the Rush Stack world. Can you remind me why that's the case?

I suppose there are few reasons:

1. In practice, path mappings are defined in tsconfig.json, which means other tools like Jest and Webpack and ts-node and Mocha would need their own corresponding configurations. (Assuming they support it at all -- can GitHub previews follow mapped paths? Probably not!) In a monorepo where projects may build using many different toolchains, this is a nuisance.

2. If mapped paths end up in a library's API surface, I think that means library consumers must re-declare the mappings somehow. (?) We could stipulate that remapping must only be used internally within a project, but that leads into...

3. In principle, we prefer for things to be standardized and explicit. In a monorepo ecosystem, we aim to break down barriers between partner teams. It should be easy for any person to contribute to any project, even if they've never seen it before. When someone sees an import statement, it should be immediately obvious what's being imported. They shouldn't need to rely on IntelliSense or go picking through a tsconfig.json file to figure out whether foo is an external NPM package or a remapped local file.

[zelliott]

Thanks for sharing these points - responses inline.

1. In practice, path mappings are defined in tsconfig.json, which means other tools like Jest and Webpack and ts-node and Mocha would need their own corresponding configurations. (Assuming they support it at all -- can GitHub previews follow mapped paths? Probably not!) In a monorepo where projects may build using many different toolchains, this is a nuisance.

Agreed that there's complexity here. Although there are benefits for developers too:

• Easier to move files around (e.g. no need to update import paths, although I'm aware some editors do this for you now).
• Easier to write import statements (e.g. no need to remember you're 4 directories deep when importing a module, everything is relative to a common base directory).

2. If mapped paths end up in a library's API surface, I think that means library consumers must re-declare the mappings somehow. (?) We could stipulate that remapping must only be used internally within a project, but that leads into...

Do you have an example of how this might happen? I don't work often in open source TS development or in the Rush Stack ecosystem so I'm not the best person to come up with one. But my understanding is path mappings are project-internal? If a package with path mappings ships its directory structure as part of its API (e.g. by having multiple entry points), other packages don't need to use those same path mappings. But I'm not confident of this.

3. In principle, we prefer for things to be standardized and explicit. In a monorepo ecosystem, we aim to break down barriers between partner teams. It should be easy for any person to contribute to any project, even if they've never seen it before. When someone sees an import statement, it should be immediately obvious what's being imported. They shouldn't need to rely on IntelliSense or go picking through a tsconfig.json file to figure out whether foo is an external NPM package or a remapped local file.

Agreed that these are good things to strive for in monorepos. But I wonder if these could also be accomplished by all packages within a monorepo using the same path mappings and these path mappings being minimal & sensible in their own way. This is basically the state of the monorepo I work in. We have a single trivial path mapping so that all imports are from the same root path, and then a small set of additional path mappings for special types of 3P imports.

[octogonz]

3. If mapped paths end up in a library's API surface, I think that means library consumers must re-declare the mappings somehow. (?) We could stipulate that remapping must only be used internally within a project, but that leads into...

Do you have an example of how this might happen? I don't work often in open source TS development or in the Rush Stack ecosystem so I'm not the best person to come up with one. But my understanding is path mappings are project-internal? If a package with path mappings ships its directory structure as part of its API (e.g. by having multiple entry points), other packages don't need to use those same path mappings. But I'm not confident of this.

Here's a simple experiment, which compiles with "baseUrl": "./src" in tsconfig.json:

src/a/A.ts

export class A { }

src/b/B.ts

import { A } from 'a/A';
export class B extends A { }

src/index.ts

export * from 'a/A';
export * from 'b/B';

It produces declarations like this:

lib/index.d.ts

export * from 'a/A';
export * from 'b/B';

lib/b/B.d.ts

import { A } from 'a/A';

Now if you publish that as an NPM package called example, then a library consumer will try to do this:

external-consumer.ts

import { A } from 'example';

It will fail because the consumer's TypeScript compiler does not know how to resolve 'a/A' in the .d.ts file. That information is in the tsconfig.json of the NPM package, which is not read by the consumer's TypeScript compiler.

This seems to make path mapping hijinks untenable for an NPM package, and only usable for end consumers. If I understand right.

But I wonder if these could also be accomplished by all packages within a monorepo using the same path mappings and these path mappings being minimal & sensible in their own way.

Sure, this is fine if your monorepo has its own proprietary structure. By contrast Rush models each project in a monorepo as a conventional NPM package. This better fits the assumptions of community tools, and it also helps adoption by making it easy to migrate projects in/out of monorepos and between monorepos. Your original question was about "the Rush Stack world" -- I don't mean to imply that everyone should accept these constraints. 😊

[zelliott]

Hey @octogonz, best to take another look before merging. See #3321 (comment) for details.

[octogonz]

🚀 Released with @microsoft/api-extractor 7.21.0

[alexeagle]

In the tweet about this fix, you suggest it's to benefit Bazel projects, so naturally I'd like to understand how to use it. But the discussion here doesn't mention a Bazel use case and neither does the associated issue. Do you have any pointers to some context I could read?

[zelliott]

In the tweet about this fix, you suggest it's to benefit Bazel projects, so naturally I'd like to understand how to use it. But the discussion here doesn't mention a Bazel use case and neither does the associated issue. Do you have any pointers to some context I could read?

I can share some context here. I work at Google, and as you also used to work there I know you're familiar with Google's Bazel/TS setup. I've been working on pulling API Extractor into Google so that teams can take advantage of some of its features (e.g. API documentation generation, API golden report, etc).

When you run API Extractor on some TypeScript project, it has logic that determines which code is part of the "current package" (that it should be generating full documentation for) and which code is part of external packages (that it should ignore). Before this PR, its logic was essentially: imports to relative paths (e.g. ./path/to/file), are part of the current package, imports to non-relative paths (e.g. path/to/file) are part of an external package. I suppose this logic was chosen because imports of external NPM packages typically take the latter form (e.g. import {jquery} from 'jquery';).

This doesn't work well for Google's Bazel/TS setup, as it ignores any path mappings specified in the tsconfig (e.g. a tsconfig generated by Bazel's ts_library). So an import like import {Foo} from 'google3/path/to/foo'; would be considered external in the previous logic, and ignored by API Extractor. Given all of our paths are non-relative (i.e. w.r.t. google3/), API Extractor ends up unable to process code correctly.

This PR introduces an entirely new way of determining whether code is part of an external package that's compatible with mappings set up in a tsconfig. Now, I'm not sure if all Bazel/TS projects rely upon tsconfig path mappings like this, or just Google's. If the former, then I think it's reasonable to say that this PR is a step in the right direction for Bazel projects to be able to use API Extractor. But it's definitely not correct to say that this PR only benefits Bazel projects (i.e. there are plenty of TS projects that use path mappings without Bazel).

This is only some high-level context, and I'd love to chat more @alexeagle if you have time. You can reach me at zell [at] google [dot] com or the API Extractor Zulip chat.

[Hotell]

wondering if there are any plans to fix this ?

we recently bumped API-extractor which is causing issues for rolluping dts/ api.md generation microsoft/fluentui#28215

I reported this some time ago here #3443.

It would be great if path aliasing worked as expected otherwise this makes this tool tightly coupled to npm/yarn workspaces.