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]Support multiple modules in the same package #1596

Open
1 of 2 tasks
PissedCapslock opened this issue Oct 21, 2019 · 8 comments
Open
1 of 2 tasks

[api-extractor]Support multiple modules in the same package #1596

PissedCapslock opened this issue Oct 21, 2019 · 8 comments
Labels
enhancement The issue is asking for a new feature or design change needs design The next step is for someone to propose the details of an approach for solving the problem repro confirmed The issue comments included repro instructions, and the maintainers reproduced the problem

Comments

@PissedCapslock
Copy link
Contributor

PissedCapslock commented Oct 21, 2019

Is this a feature or a bug?

  • Feature
  • Bug

Please describe the actual behavior.

I'm looking for a way to generate documentation for a Typescript library/package which does not have a single input file.
Instead, the package contains of a set of modules where each module exports some classes, interfaces, ... . It is very similar to what Angular has, e.g. if you npm install @angular/router you receive 3 modules in the same npm package:

  • The @angular/router module
  • The @angular/router/upgrade module
  • The @angular/router/testing module

Judging by the documentation, this should be possible using api-extractor:

One significant limitation for .d.ts rollups is the assumption that your package has a single entry point. (If that’s not the case, you probably won’t be able to use this feature of API Extractor, although you can still use the API report and documentation generation features.)

I tried this, but could not get it to work. I have setup a test repository which you can use to reproduce my problem:

git clone https://github.com/PissedCapslock/tsdocexperiment.git
cd tsdocexperiment/
npm install
npm run-script apiextractor
npx api-documenter --input-folder temp --output-folder docs

which results in

Reading firstmodule.api.json
Reading secondmodule.api.json
Error: Another member has already been added with the same name and containerKey

What I do in this repository is the following:

  • The src folder contains a mix of .d.ts files and .ts files. This is probably irrelevant, but mainly done to mimic our current situation where we are migrating a .js codebase Typescript, and have manually created definition files for unported JS code
  • The tsc compiler compiles the src folder, and stores the output in the lib folder.
  • I copy the d.ts files from the src folder into the lib folder as well. That way, the lib folder contains all d.ts files
  • I run the API extractor on all the d.ts files in the lib folder by generating an api-extractor.json file where the entrypoint points to that specific d.ts file, and store that output into the temp folder

This approach results in .api.json files where the module information is gone. I only see the package name. So of course, running api-documenter on it does not work.

Is multiple entry points simply not supported (despite what is mentioned in the documentation), or am I doing something wrong ?
Looking at the documentation of the APIPackage class which has a ReadonlyArray<ApiEntryPoint> property, it certainly looks like multiple entry points are supported.
On the other hand, WorkingPackage.entryPointSourceFile contains the following comment:

  /**
   * The entry point being processed during this invocation of API Extractor.
   *
   * @remarks
   * The working package may have multiple entry points; however, today API Extractor
   * only processes a single entry point during an invocation.  This will be improved
   * in the future.
   */

which sounds less promising.

If supported, I would appreciate it if somebody could point me in the right direction. In return, I would gladly open a PR for the api extractor site with some extra documentation.

@octogonz octogonz added the repro confirmed The issue comments included repro instructions, and the maintainers reproduced the problem label Oct 27, 2019
@octogonz
Copy link
Collaborator

octogonz commented Oct 27, 2019

Notes for getting this repro to run correctly:

  • The readdirSync + withFileTypes API requires Node 10.10 or newer
  • The "compile-scripts" wildcard assumes a Unix shell (needs to be manually expanded on Windows)
  • Delete rm -rf lib && for Windows
  • The string replacements in api_extractor_experiment.ts assume / is the path separator
  • The last command needs to include markdown:

npx api-documenter markdown --input-folder temp --output-folder docs

@octogonz
Copy link
Collaborator

@PissedCapslock I'm impressed you actually got fairly far with this. 😊

The crash is happening because when you invoke api-extractor on a secondary entry point, it produces an .api.json file with the same package name as before:

{
  "metadata": {
    "toolPackage": "@microsoft/api-extractor",
    "toolVersion": "7.5.1",
    "schemaVersion": 1003,
    "oldestForwardsCompatibleVersion": 1001
  },
  "kind": "Package",
  "canonicalReference": "tsdocexperiment!",
  "docComment": "",
  "name": "tsdocexperiment",   // <==============
  "members": [

...and then API Documenter is complaining because it doesn't expect to load two different definitions for the same package.

@octogonz
Copy link
Collaborator

One possible workaround would be to generate the "name" field differently. For example, you could replace this:

    // Run the API Extractor command-line
    const extractorConfig: ExtractorConfig = ExtractorConfig.loadFileAndPrepare(tempApiExtractorConfigFile);

...with this:

    // Run the API Extractor command-line
    const configFile: IConfigFile = ExtractorConfig.loadFile(tempApiExtractorConfigFile);

    const extractorConfig: ExtractorConfig = ExtractorConfig.prepare({
      configObject: configFile,
      configObjectFullPath: path.resolve(tempApiExtractorConfigFile),
      packageJson: {
        name: path.basename(definitionFile).replace('.d.ts','')
      },
      packageJsonFullPath: path.resolve("package.json")
    });

But I'd like to think about this a little more and see if we can maybe support it at least partially. Frankly I've never really attempted it.

@PissedCapslock
Copy link
Contributor Author

I've been thinking about changing those name fields, but I think I will have to do more than that. When linking from "one package config" to another (e.g. class Foo in module A has an input argument of type Bar from module B), the link in the resulting JSON will need to know the package name of A and B to link correctly.

To give a bit more context (as this is clearly a request for a new feature and not a simple bug report) about why I was trying this.

At our company, we sell a very large, closed-source, commercial JS library (about 250 public classes in the API). We started with this library years ago, and back then, AMD modules were the way to go. The result is a library where we had:

  • A single AMD module per file
  • Each AMD module exported a single class
  • The classes have constructors, or in some cases only a bunch of static methods

The main reasons to ship it like this are:

  • We want to ensure that customers only need to load what they actually use in their application. The size of our whole library is over 10MB, so always loading everything is not an option. By having a lot of tiny AMD modules, there was no need to rely on tree shaking tools (which might or might not work), customers only imported what they needed.
  • From an API point-of-view, splitting it up in different namespaces and classes made sense.

As we are originally a Java shop, we offered documentation very much in the style of Javadoc:
image

Now we are looking into migrating our internal code base to Typescript (might take some time), and extracting documentation from the Typescript code. While tree shaking tools have improved over the years and work pretty well with ES6/Typescript modules, we like to stick to our approach of having a bunch of very small modules (all contained in the same package however).

This is clearly a use-case that most npm packages do not encounter, as those are very tiny libraries in general.

@octogonz
Copy link
Collaborator

This issue is a dupe of #664

@octogonz
Copy link
Collaborator

octogonz commented Oct 28, 2019

@PissedCapslock The difficulty of implementing this feature breaks down into 3 levels:

  1. Easy: Support for documentation and API reports
  2. Medium: Support for isolated .d.ts files
  3. Complex: Support for .d.ts files with shared declarations

To understand the hard case, suppose we have two API entry points src/simple and src/containers:

src/simple.ts (entry point)

import { BaseControl } from './common';

/** @public */
export class Button extends BaseControl { }

src/containers.ts (entry point)

import { BaseControl } from './common';

/** @public */
export class TabPanel extends BaseControl { }

src/common.ts (NOT a public entry point)

/** @public */
export class BaseControl { 
  private _name: string;
}

When we create the two .d.ts rollups, where do we put BaseControl? We cannot duplicate it into both simple-rollup.d.ts and containers-rollup.d.ts. Because of the private member, the TypeScript compiler will consider the two copies to be different, incompatible types.

Instead, API Extractor would need to collect any symbols that get exported by more than one entry point, and move them into a synthetic internal-rollup.d.ts file that is NOT an official entry point for consumers. Then the rollups can use import statements referring to internal-rollup.d.ts.

There's also a puzzle of how to represent this on the documentation website. Presumably the table of contents would show something like this:

  • my-library package
    • simple-rollup entry point
      • Button class
    • containers-rollup entry point
      • TabPanel class
    • common declarations <--- not a real entry point!
      • BaseControl class

And what if src/simple.ts and src/containers.ts both export the BaseControl class? Then it would need to be represented as an "import alias", like this:

  • my-library package
    • simple-rollup entry point
      • Button class
      • BaseControl import <--- tiny stub page
    • containers-rollup entry point
      • TabPanel class
      • BaseControl import <--- tiny stub page
    • common declarations
      • BaseControl class <--- the full docs with all class members

The design of the full-blown feature is fairly clear, but as you can see there's a fair amount of plumbing involved.

Whereas, if you only need the partial "Support for documentation and API reports" from my list above, then it might be a lot less work.

@octogonz octogonz added enhancement The issue is asking for a new feature or design change needs design The next step is for someone to propose the details of an approach for solving the problem labels Nov 4, 2019
@Feiyang1
Copy link
Contributor

Feiyang1 commented Apr 30, 2020

I'm also very interested in this feature as some libraries I maintain have(will have) multiple entry points. I'm glad that you think it's easy to add support for documentation and API reports which , I think, are most important to most people. For types and sharing types, we can just use the ones generated by tsc.

@octogonz Have you started working on this? If not, would you accept contribution? I'd like to give it a shot at the documentation and api report part.

@octogonz
Copy link
Collaborator

@octogonz Have you started working on this?

Nope!

If not, would you accept contribution?

Absolutely! 😁

I'd like to give it a shot for the documentation and api report part.

There's some onboarding docs here: https://api-extractor.com/pages/contributing/building/

Also I'm usually reachable on Gitter for specific questions.

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 needs design The next step is for someone to propose the details of an approach for solving the problem repro confirmed The issue comments included repro instructions, and the maintainers reproduced the problem
Projects
Status: AE/AD
Development

No branches or pull requests

3 participants