Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[api-extractor] Improve AstReferenceResolver to resolve references to external packages #1195

Open
octogonz opened this issue Mar 29, 2019 · 2 comments
Labels
enhancement The issue is asking for a new feature or design change

Comments

@octogonz
Copy link
Collaborator

octogonz commented Mar 29, 2019

In API Extractor 6, when an @link or @inheritDoc tag referred to a declaration in an external package, it was resolved by loading up the .api.json file from that package's folder (found under node_modules). This required the .api.json files to be published with the NPM package, which had a number of disadvantages:

  • The JSON files are large, increasing the size of the NPM tarball
  • If the JSON files are published, then they must not contain information about internal/alpha APIs, since it may pose a confidentiality issue for closed-source software. This means @inheritDoc cannot refer to such definitions.
  • In general, @inheritDoc also cannot refer to any API that is not a public export

In API Extractor 7, we now have the technology to resolve external references by analyzing the .d.ts files directly, which solves all of the above problems. However this feature was cut from the initial release of AE7 to avoid holding up its release. Instead, as a temporary mitigation, it handles external references as follows:

  • If an @link tag refers to an external package, API Extractor does not perform any link checking, and API Documenter will simply remove the hyperlink from the docs

  • If an @inheritDoc tag refers to an external package, API Extractor does not process it. But API Documenter will process it, if it can find the target in one of its .api.json files.

After AE7 ships we need to come back and improve AstReferenceResolver.ts to handle external references. There are a couple interesting aspects:

  1. When we encounter {@link some-package#SomeClass}, there can be more than one match for some-package in the node_modules folder. Which one we choose depends on the location of the source file that contains the reference.
  2. If the package is not referenced by any .d.ts file in the input set, then we need to ask the compiler to consider a new source file outside of its original scope of analysis.
@octogonz octogonz added the enhancement The issue is asking for a new feature or design change label Mar 29, 2019
@tylerbutler
Copy link
Member

Does this mean that inheritDoc doesn't work at all across packages? Does api-extractor emit any warnings or errors in this case? I have observed that my API reports say that methods are undocumented if they inheritDoc from an external package, which makes it really tough to document implementations of abstract base classes, for example.

Is there any workaround? If not, I would suggest that this be documented as a limitation (I found nothing other than this issue) and emit warnings of some kind when api-extractor is executed.

@renoirb
Copy link
Contributor

renoirb commented Sep 2, 2021

Yeah, I've tried to do something similar and I get WARNING: Unable to resolve reference in the following.

For example, when we need that has type definitions that wants to use RushConfiguration's rush.json schema.

import { RushConfiguration } from '@microsoft/rush-lib'

/**
 * {@inheritDoc @microsoft/rush-lib#RushConfiguration.rushConfigurationJson}
 * @internal
 */
export type IRushConfigurationRepository = NonNullable<
  Required<RushConfiguration['rushConfigurationJson']['repository']>
>

/**
 * GitHub repository configuration for this pipeline
 * All normalized taken from what Jenkins can provide and what
 * RushJS.io can also give us.
 *
 * @public
 */
export interface IRushMetaGitHubRepo {
  /**
   * The branch to use to make releases on.
   * That is the branch name that Rush will use to push release tags and be used to build and publish to Nexus.
   *
   * @remarks
   * Refer to {@link IRushConfigurationRepository | rush.json at `repository.defaultBranch` }
   *
   * @defaultValue
   * The default is `master` unless monorepo's rush.json at repository.defaultBranch is set differently.
   */
  defaultBranch: string
  /**
   * Name of the authoritative Git repository
   *
   * @remarks
   * Refer to {@link IRushConfigurationRepository | rush.json at `repository.defaultRemote` }
   *
   * @defaultValue
   * The default is `origin` unless monorepo's rush.json at repository.defaultRemote is set differently.
   */
  defaultRemote: string
  /**
   * What would be `env.GITHUB_REPO_URL_WEB`
   *
   * Git repository over HTTP
   *
   * @example
   * ```
   * 'https://github.com/my-org/my-monorepo-example-project'
   * ```
   *
   */
  urlWeb: string
  /**
   * What would be `env.GITHUB_REPO_URL`
   *
   * Git repository DSN, in the format of a git at sign, not an HTTP URL.
   *
   * @remarks
   * Refer to {@link IRushConfigurationRepository | rush.json at `repository.url` }
   *
   * @example
   * ```
   * '[email protected]:my-org/my-monorepo-example-project.git'
   * ```
   */
  url: string
}

I've tried many variants of the following, there's always a warning.

Also, I feel that the MUST HAVE a reference in import type { ISomething } from '@external/package' just for TSDocs would be an anti-pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is asking for a new feature or design change
Projects
Status: AE/AD
Development

No branches or pull requests

3 participants