[api-extractor] Support "import * as __ from __" for local paths

[octogonz]

PR #1002 adds support for declarations like this:

import * as React from 'react';

This is a trivial implementation, where we pretend that * is an actual name exported by the "react" package. It does not work for the general form that introduces a new namespace, for example:

// For example, "Button" in controls.ts becomes "controls.Button"
import * as controls from './controls';

This form is called ts.SyntaxKind.NamespaceImport because it is essentially a more general form of the namespace controls { } declaration kind.

As such, it should probably produce an ApiItemKind.Namespace item.

[octogonz]

@gf3 shared a branch gf3/api-extractor-issue@30de59d that repros this issue.

[octogonz]

Some other examples were given in the comments for issue #663

[shlomiassaf]

@pgonzal I think that those imports should be handled differently, from a usage perspective and rather then semantic...

import * as React from 'react';
import * as controls from './controls';

export class ABC {
  p1: React.Component;
  p2: controls.Control;
}

The code above is similar to this:

import { Component }  from 'react';
import { Control } from './controls';

export class ABC {
  p1: Component;
  p2: Control;
}

The local import is purely semantic...
When generating docs it doesn't make sense to render p2: controls.Control; so the library should find the direct reference and ignore the namespace.

When importing from third party, it's a different story and the way it is rendered is up to the user, just need to make sure there's enough info...

[octogonz]

@shlomiassaf

FYI API Extractor wants all reachable types to have exported names, mainly for the software engineering reason that it's more difficult to use an API with anonymous types, and also more difficult for the documentation to talk about it.

For your second example, API Extractor should warn about Component, something like this:

import { Component }  from 'react';
import { Control } from './controls';

export class ABC {
  // WARNING: the declaration "Component" is not exported
  p1: Component;
  p2: Control;
}

Whereas in your first example, it should issue this sort of warning:

import * as React from 'react';
import * as controls from './controls';

export class ABC {
  // (no warning because the "React" module is an external package)
  p1: React.Component;

  // WARNING: the declaration "controls" is not exported
  p2: controls.Control;
}

If you mixed them, then the desired behavior is less clear:

import * as React from 'react';
import * as controls from './controls';
export { Control } from './controls';

export class ABC {
  // (no warning because the "React" module is an external package)
  p1: React.Component;

  // WARNING: the declaration "controls" is not exported
  p2: controls.Control;
}

I suppose the ideal behavior might be to realize that controls.Control is a synonym for Control and normalize it to eliminate the controls. part. But this is technically difficult. I haven't thought about it in depth because this problem seems to never occur in any of our code bases heheh.

[shlomiassaf]

@octogonz Thanks for clear explanation.

I think that the underlaying issue is more about the concept and direction of this project.
This tool is used for several tasks, document generation is one of them.

We can agree that generating documentation from the json output can happen is a lot of ways, so many that I can't count :)

At it's current state, the project does provide a way to change the behaviour, configuring it, etc...

Taking a lot of assumptions is risky because it means that it will only do and support things that occur in your code bases and that's not a very good approach for a public project.

I see great importance in tsdoc and this tool and I think that wide adoption will help the community because there are a lot of tools for this job, each with pros/cons and having a unified tool from MS is probably the best solution.

But, for that we need flexibility, understanding that your code base might not be the same as another's code base.

For example, let's say I built an angular library that uses a lot of symbols from various @angular libraries and also symbols from rxjs

Now of course all of them are not exported, but in my doc generator I can decide to generate links for them, based on some schema... links to the external documentation for these libraries...

So if a new ApiItem is created hosting the (not) exported symbol with it's package name, the doc generator can do the magic...

In angular it's a very simple schema: https://angular.io/api/[PACKAGE NAME]/[SYMBOL NAME]
Examples:

import { HttpHeaders } from '@angular/common/http';

https://angular.io/api/common/http/HttpHeaders

import { Component } from '@angular/core';

https://angular.io/api/core/Component

Usually, since documentation is auto-generated, a schema always exists...

[jeremymeng]

We are currently using the following pattern. Is there a recommended workaround?

import * as Models from "../lib/generated/lib/models";
...
export { Models };

[octogonz]

Replying to @shlomiassaf (sorry I didn't see this comment before):

I think that the underlaying issue is more about the concept and direction of this project. [...]
But, for that we need flexibility, understanding that your code base might not be the same as another's code base.

In your example, Angular is one library with a self-contained opinionated representation. Whereas API Extractor's input is a collection of type signatures that are scattered across various .d.ts files, source folders, and external packages, sometimes with different conventions or compiler configurations. API Extractor starts by transforming these inputs, reducing them to a consolidated signature for the library, formulated to be understandable by humans. This is basically what you see in the .d.ts rollup file. The transformation is tricky and somewhat subjective. API Extractor will probably never be able to handle every last exotic TypeScript construct that someone might try to use. However we do aim to support every "reasonable" construct, for example cases that are very popular or don't have easy workaround. In the past year we have made some major steps towards this goal (e.g. enabling support for projects that had no hope of working under API Extractor 6.x). But obviously there's a lot left to do. We'd greatly appreciate community help with that, although the core analyzer code is somewhat complicated and definitely not the easiest area to get started with.

As far as the direction of the project, I'll also mention that we are /not/ seeking to decouple the three use cases (documentation, API reporting, and .d.ts rollups). People frequently ask about this, but these problems are very closely connected, and there lots of advantages to tackling them together. API Extractor 7.x was a big rewrite to make the 3 generators share a common analysis, and it really improved the overall architecture. This "integrated solution" is I believe a major strength of API Extractor compared to other tools.

We are currently using the following pattern. Is there a recommended workaround?

import * as Models from "../lib/generated/lib/models";
...
export { Models };

@jeremymeng Currently the only workaround is to somehow declare your namespaces using the namespace keyword. (I very much want to fix issue #1029, but it's not a simple fix. It may require several days of work or more. I can't tackle that right away, but it's definitely a high priority.)

[gf3]

is there anything we can do to help move this along? we're hoping to use this tool at Shopify to build out some of our public facing documentation for a few libraries and tools

[mterrel]

@octogonz After looking at all the similar tools out there for TypeScript projects, this definitely seems like the right choice. But we do use the same pattern as @jeremymeng, so we're blocked by this issue.

I'd rather pitch in and help out here than spend time setting up a different tool, only to come back to api-extractor later. So I'm definitely willing to put in some work to fix this. I've got quite a bit of experience working with the TS compiler API and the AST, but am new to api-extractor. Reading through the previous comments, it sounds like the fix may be a bit involved, so I thought I'd reach out first to see if 1) you'd like some help here and 2) if you've got any high level guidance that you'd like me to follow. I'm happy to jump on Slack or a call or whatever is easiest if you want to chat.

[octogonz]

@mterrel That would be great and much appreciated! Ping me on Gitter and we can figure something out. Very exciting!

[xirzec]

Any chance this is still being worked on? It would be lovely to have for the Azure SDK.

[adventure-yunfei]

Blocking upgrading from AE6 by this, too...

The syntax export * as something from "./localfile works on older api-extractor version (tested on 6.1.3), but fails on latest version (7.4.2).

I remember that I should have fixed this case by #773. Is this a regression caused by AE7 refactor?

[octogonz]

Is this a regression caused by AE7 refactor?

@adventure-yunfei Yes, in AE6 the .api.ts report was created using the original analysis engine, whereas the .d.ts rollups were created using the newer (more accurate) analysis engine. Thus, in AE6 the export * as something from "./localfile" worked for the API report, but it was broken when generating .d.ts rollups. (The example that you shared does not encounter this problem because .d.ts rollups are disabled.)

In AE7 everything, now uses the newer engine. The older engine is completely removed. That is why this problem has resurfaced. Fixing it is somewhat more difficult, because the solution needs to produce a correct .d.ts rollup.

I don't have time to work on this myself right now (I'm about to start work on some new Rush features), but if someone wants to volunteer, I'd be happy to help with the design.

Here's some initial thoughts.

Interesting edge cases

We need to enumerate the weird edge cases, and come up with a design for how we would represent them in the .d.ts rollup. For example, consider this pathological case:

import * as ns1 from './File1';
import * as ns2 from './File1';
import { thing } from './File1';

export { ns1 };
export { ns2 };
export { thing };

What happens to the string ./File1 when we rolls that up into a single rollup.d.ts file?

It's perfectly okay for our answer to be that "API Extractor supports import * for certain common usages, but certain other usages are NOT supported because (1) they cannot be rolled up reasonably and/or (2) they can be easily rewritten to better forms." It's okay to impose constraints. However, if we do that, it /is/ important to be able to clearly state what is and isn't supported. The rule can't be vague.

This exercise will also lead us to understand how to model ns1. Is it a conventional namespace (i.e. AstNamespace? Or is it some new construct? (These might be easy answers BTW; I haven't had time to think about it in any depth.)

Getting started

With the new analyzer, the starting point is in the _tryMatchImportDeclaration() and _tryMatchExportDeclaration() methods, which parse the import and export statements and decide what to do with them.

The website has some technical notes about the new architecture that might he helpful, along with debugging instructions.

If someone wants to pursue this, ping me on Gitter and I can provide more help.

[transitive-bullshit]

Just ran into this on notion-kit.

@octogonz I can confirm that 7.8.2-pr1796.0 resolved my issue.

[CoreyDotCom]

Would be great to get this merged... fix is very helpful.

[ghost]

It has been standardised.

It's still in the ECMAScript 2021 draft.

It's in the TypeScript 3.8 release. wink

That release still doesn't make it a standard feature. 😛

I'm actually a bit confused, then. Considering how this is a library designed for TypeScript, if it's a released TypeScript feature, isn't that what matters?

[octogonz]

I'm actually a bit confused, then. Considering how this is a library designed for TypeScript, if it's a released TypeScript feature, isn't that what matters?

This seems like a pedantic concern. Regardless of the technical definition of "standard", if TypeScript supports a particular language feature, and using it is considered a best practice, then API Extractor will do its best to support it.

[hiranya911]

Is this issue also going to address problems like #2145 where API Extractor does not include re-exported namespace content in the API report? Fixes in 7.8.2-pr1796.0 allows me to generate a report, but it doesn't include any of the re-exported APIs.

[jasonswearingen]

7.8.2-pr1796.0 kind of works, but I have to disable apiReport

[jasonswearingen]

there is another, similar problem with export * as moduleName from "./moduleName" which gives the generic error message:

Internal Error: Unable to parse module specifier

the 7.8.2-pr1796.0 does not work with the export * style

[hiranya911]

Just ran into this while trying to re-export types declared in another module:

import * as _app from './app/index';

export namespace credential {
  export type Credential = _app.Credential;
}

It seems, at least for cases like the above you can workaround the error by doing something like this:

import { Credential as _Credential } from './app/index';

export namespace credential {
  export type Credential = _Credential;
}

[IgorMinar]

@octogonz thanks for looking into this problem. It seems that #1796 is a solution that is already in a pretty good shape. Could you please help us get it across the line and resolve this problem?

Fixing this issue would significantly help Angular (angular/angular#42064) where we heavily use namespaces to reduce the size (and associated memory footprint) of generated d.ts files.

[gregjacobs]

@IgorMinar @octogonz +1 for this - we just upgraded to Angular 12 and can no longer use API extractor 😢

[octogonz]

Also, please support export * as util from './utils' syntax

This syntax is not handled by PR #1796. It is a relatively easy addition, but in the interest of getting that PR finally merged, I'm going to postpone export * as to later PR. I've created #2780 to track that.

[octogonz]

PR #1796 was released with API Extractor 7.17.0

@IgorMinar @gregjacobs Could you try it with Angular 12 and confirm whether that issue is resolved? If not, please create a new GitHub issue with repro steps for Angular. Thanks!

[IgorMinar]

We will. Thank you!

[alan-agius4]

Hi @octogonz, can confirm that 7.17.0 addresses the issue with Angular.

[octogonz]

Hi @octogonz, can confirm that 7.17.0 addresses the issue with Angular.

Awesome! 🎉