Support export declarations

[lddubeau]

[This PR passes all its tests when run with typedoc-default-themes incorporating that PR. Since there is no version of typedoc-default-themes published with that PR, typedoc-default-themes has to be built and installed manually in order for the suite to pass. This is why the Travis tests currently fail.]

This PR aim to provide support for export declarations.

Aims

1. Support export declarations.

2. When typedoc is used for producing documentation that covers everything (no flags are used to exclude anything), the documentation should immediately make clear what symbols are exported and what symbols aren't. (This is why the changes I made show an [Export] label in front of exported symbols or [x] in index entries for exported symbols.)

3. Keep it simple.

Some of these aims work against each other, unfortunately. (For instance, simplicity would be maximized by not supporting export declarations at all.) The end result is a relative balance fo the aims rather than a binary "satisfied"/"not satified" checklist.

Prior to this PR typedoc ignored export declarations. Export declarations are export statements that declare that a symbol is exported without at the same time defining what the symbol is.

This, for instance, is not an export declaration, but a variable declaration with an export flag:

export const x = 1; 

This is not an export declaration either, but an export assignment:

export = foo; 

These are export declarations:

// Export a local symbol. 
export { foo }; 
// Export a local symbol under a different name. 
export { foo as bar }; 
// Export all the symbols from another module. 
export * from "other/module"; 
// Export a symbol from another module. 
export { foo } from "other/module"; 
// Export a symbol from another module under a different name. export { foo as bar } from "other/module"; 

Core Changes

Providing support for these declarations required substantial changes:

1. The PR introduce a distinction between a new reflection flag named Export and the pre-existing Exported flag. Export means that the reflection has been directly exported either because it is declared with the export flag, or it has been subject to an export declaration. For instance, in this example:

   export const x = 1; 
   const y = 2; 
   const z = 3; 
   export { z }; 

   x and z have the Export flag, but y does not.

   In contrast Exported means that a symbol is going to be visible in the final documentation either because it is directly exported (like x and z in the above example), or because it is a child of a parent structure which is exported. A prime example of symbols that would have Export false but Exported true is the members of an exported class.

   export class Foo {member: number = 1;} 

   In the example above Foo has Export and Exported true, but member has Exported true and Export false.

   The two flags are necessary because they serve different purposes.

2. Previously, when --excludeNotExported was used, the converter would simply skip producing declarations for symbols that would have Exported false. This PR changes the stragegy so that all declarations are initially created whether Exported is true or not. After the whole tree is created, the converter than walks the tree to eliminate those declarations that are not Exported.

   This allows handling this scenario:

   // more stuff...
   
   export { /* ... */ , x, /* ... */ } 

   We don't know when we run into x whether it will later be exported so we cannot just skip creating its declaration. It has to exist so that when we process export {...} there's a reflection for x that can have its Export flag turned on.

3. Export declarations are essentially created as stubs in the first pass that creates the tree of reflections, and then in a second pass they are transformed into DeclarationReflection objects. I don't think there was an existing exact parallel to this prior to this PR. This is necessary to handle things like this:

   export { a, b }
   
   const a = 1; 
   const b = 2; 

   It is entirely valid TS to export a symbol before knowing what that symbol is. export { a, b } should result in the declarations for a and b having their Export (and Exported) flags turned on, but by the time export { a, b } is processed, neither of the two declarations exist. So it is not possible yet to act on them.

4. This PR introduces the notion of declarations that "rename" other declarations. This is to handle things like:

   export { foo as bar }; 
   export { something } from "other/module"; 
   export * from "other/module"; 

   The last two cases are treated as renames because they provide alternate load paths to what is ultimately the same symbol.

Renames

A long parenthesis about how renames could be handled in the final rendering. I have moved back and forth between a) a solution that would produce handle a rename of foo to bar (export { foo as bar}) by copying the documentation for foo, with the name foo replaced with bar, and b) a solution that documents bar with a link to foo. Just to clarify what I'm talking about, suppose the code

const x = 1;
export { x as y } 

With the copying approach, the documentation would contain something like this:

[Const] x
const x = 1

[Export] [Const] y
const y = 1

y would get its own complete entry, separate from x and standing on its own.

With the linking approach, the documentation instead contains:

[Const] x
const x = 1

[Export] y
Renames [x](hyperlink to x)

The documentation for y states that it is exported but is otherwise just a reference to x.

The copying approach is relatively simple for const x = 1 but it gets more complicated when you're dealing with more complex structures. It turns into a deep cloning operation, followed by a rename, and you quickly run into special cases and exceptions. For instance in the case of a function declaration you need not only rename the top level declaration of the function but also the function signatures that appear in it. There are also issues with providing a proper context for the clone, and providing sensible references to the source code, etc. These are most likely surmountable but may ultimately require significant redesign of the internals.

Moreover, even if the vagaries of cloning are handled, there's at least one additional major hurdle to the copying approach: a declaration that is copied and then renamed may make the human-produced parts of the documentation difficult to understand or downright nonsensical. For instance, suppose the class Foo is documented with something like this:

If so and so, then you should create a Foo with such and such
parameters. Do not use Foo when blah [...]."

If you want to frobulate the fnord, then you must do it like this:

const foo = new Foo("blah", 1, { q: "eeek" }); 
foo.frobulate(FROBULATE_SLOWLY); 
foo.getThisValue("fnord"); 

And now suppose that typedoc copies declaration for Foo that renames it to Bar because of export { Foo as Bar}. All the documentation generated by typedoc will be fine, but the human-written documentation will still refer to Foo. The user will be confused when reading that documentation attached to Bar. There's no trivial way to work around that. The linking approach dodges this issue entirely.

Theme Changes

THIS PR INCLUDES CHANGES THAT REQUIRE THEME UPDATES:

1. Reflections no longer decide whether a flag is relevant for rendering or not. All flags are stored in the flag array of reflections, and it is up to the theme to decide what the theme considers relevant.

2. Declarations that rename other declarations (discussed above) need special handling by themes.

I'm going to submit a parallel PR for typedoc-default-themes that takes care of changing the default theme to handle the changes above.

The changes I've made to default theme to handle the change in typedoc's core code are as follows:

1. When symbol X has the Export flag,

a. X has an [Export] label in the declaration produced for X.

b. When X is listed in indices, it has an [x] label.

(This allows immediately knowing whether a symbol was exported by the export keyword or an export declaration, or not.)

2. When a symbol renames another, its documentation details are reduced to the following:

   Renames [foo](link to foo).

Misc Notes

1. The rendering test case for examples/src/basic/enumerations.ts is wrong. The Size module is merged with the Size enumeration. The symbols in the Size module are considered to be exported even though they are not actually exported from their module. This is primarily a merging problem, which is outside the scope of what this PR was aiming to fix. (To be clear: the problem existed prior to this PR and continues to exist after it.)

2. There is more potential for linking problems after this PR when users use the --excludeNotExported flag. Consider export { foo as bar } If --excludeNotExported is used and foo is not exported, then the documentation for bar cannot refer to foo, because it is removed from the tree of reflections. The current solution is to give a warning indicating the issue and letting the user decide whether to drop --excludeNotExported or work with invalid documentation.

   I don't think a lot of time should be spent trying to find a more elegant solution. Of the five types of export declaration I listed at the beginning of this comment only the "Export a local symbol under a different name" type is liable to cause dangling renames. The export declarations with a from clause are not liable to cause dangling renames because the symbols they rename are necessarily exported from the other module. The case export { foo } is also not liable to cause a dangling rename because it does not result in a rename. Instead, the Export flag is turned on for the declaration of foo. (For instance, const foo = 1; export { foo } is equivalent for this discussion as if it had been export const foo = 1;) I've never encountered a case where the "Export local symbol under a different name" type of export declaration was justified as a matter of good coding practice. If you're going to do this:

   function foo() {} export { foo as bar } 

   You might as well just do:

   export function bar() {} 

3. As described above, in the index that appears at the top of a module, the symbols that are exported get an [x] label in front of them so that someone reading the index can immediately know what is exported and what isn't. I'd prefer some sort of icon. However I see the icons are stored in a PhotoShop file. I do not have the software needed to edit it, nor am I proficient with such software. (Here's my 2 cents: the themes should move away from this setup in favor of using something like FontAwesome instead.)

[aciccarello]

Thanks for the PR!

If anyone sees this, I'd appreciate help reviewing the PR.

[bkniffler]

Why is this kept open for so long? I would love to see the changes merged.

[aciccarello]

@bkniffler This PR has been delayed because (1) it is large and involves theme changes so it take a lot of volunteer time to review (2) it has some merge conflicts. I'd love to get this merged but haven't had the time to work through it. Any help reviewing is appreciated.

[lddubeau]

@aciccarello There were no conflicts back when I initially submitted this PR. I've rebased it just now. As we speak it is rebased on top of PR #1006 because the test suite won't run without that fix. It is actually while I was rebasing this PR that I discovered the problem with CommentPlugin not resetting its state properly.

Travis failed again, but that's to be expected. As I said in my initial post:

This PR passes all its tests when run with typedoc-default-themes incorporating that PR. Since there is no version of typedoc-default-themes published with that PR, typedoc-default-themes has to be built and installed manually in order for the suite to pass. This is why the Travis tests currently fail.

---

It is extremely frustrating that 10 months after the PR has been submitted no work has been done towards merging it. The current lack of support for export declarations is a blocking issue for most of my projects. I've been telling people to clone the repositories and read the documentation comments in the source.

[aciccarello]

@lddubeau My sincere apology for the extended delay in getting this merged. I've only recently been able to make progress on some of the theme impacting changes. This is at the top of my priorities for big features to get merged. Thanks for keeping it updated.

[gnidan]

Hey, anything happening here?

[vakrilov]

Hey folks - any progress on this PR?

[nhynes]

I pulled this branch, but found that a structure like

index.ts
  module/
    index.ts
    a.ts

with

// :/index.ts
export * from 'module'

// libA/index.ts
export { A } from './a'

produces the correct symbols, but the URL link for A goes to /classes/_index_.a.html instead of /classes/_module_.a.html

[gnidan]

If one of the maintainers can identify next steps here, I'm sure there are plenty of us in the community who'd be willing to assist!

[Gerrit0]

There are a few things we need to do to get this merged:

1. Verify that links work correctly - Support export declarations #801 (comment)
2. Add a test to ensure 1 is fixed
3. Review current tests to ensure every form of export / import is tested & handled correctly

I thought I'd have enough time to get started on this this weekend, but didn't get to it. Hopefully next weekend.

[Gerrit0]

Well, turns out step 0 had to be done first - get everything merged up to master. This took considerably longer than I expected it would, so I'm basically out of time this weekend... I should have enough time next weekend to actually look into step 1 above.

Tests pass locally, but will still fail on Travis since they depend on theme changes.

[gnidan]

Thank you for taking the time @Gerrit0, sincerely. I am guessing that synchronizing a branch is not your favorite way to spend a weekend, to put it lightly.

If you point me in the direction of the existing tests (or just provide a few examples of "this syntax should be understood to mean these symbols" or such), I can compare with my notes for specific import/export syntaxes that I ran into trouble trying to use.

[alalonde]

@gnidan Take a look at my PR (#1103). I added a test as part of that, so you can see the added/modified files associated with a new test. Poke around the src/test/converter folders, there's one for each specific case, and some addressing multiple cases.

[Gerrit0]

@alalonde I've added your test to this PR. It caught a few cases missed previously, so I've added some code to fix that as well.

Tomorrow I'll hopefully have time to look into links.

[gnidan]

Awesome @alalonde and @Gerrit0, I'll review that this week to see if you're missing any case that I'm aware of!

[Gerrit0]

@nhynes I've finally gotten around to building this and playing around with it a bit... you are correct that the links go to _index_.a.html, we also apparently now create a "renamed from" page that links to the original class. I haven't been able to reproduce any broken links. Am I doing something wrong?

[nhynes]

@Gerrit0 thanks very much for looking into this. The links were never broken--just redirecting to an empty index page representing the re-export. Linking to the original item sounds like a reasonable fix.

[Gerrit0]

There's a problem with linking directly to the original item - renames.

If I click on Bar in a module that contains export { Foo as Bar }, I'll be very confused when the documentation describes Foo and likely assume I followed the wrong link.

I'm open for suggestions to avoid this, as it would be nice to not have all the extra pages. I haven't been able to come up with a good solution.

[Gerrit0]

Since I've been using this PR to track work on the feature, here's where I'm at:

There are 3 PRs & 1 fork providing different types of support for imports/exports.

iRath96 - The first attempt I'm aware of, adds a wildcardImports property to ContainerReflection that tracks export * from statements, converts imported reflections as if they belong to the current module. No support for HTML output (unless I missed something)
#646 - Another implementation that treats imported symbols as if they are declared in the module they are imported into. This results in multiple copies of the documentation. Introduces an importedFrom property on DeclarationReflection that points to the imported reflection & is used to link to the original reflection. Somehow avoids the need to defer resolution.
#801 - This PR, defers resolution until after all other conversions have been done & produces renames that link to the original reflection. Does not doubly convert symbols, but does produce another problem. Previously, when --excludeNotExported was passed, TypeDoc would skip non-exported symbols, which provided a performance benefit. With the deferred resolution, we can't do that anymore (or rather, I think we can but the current implementation doesn't)
#1079 - Similar to #646, copies reflections. I haven't spent enough time comparing them to say more.

---

• I'd like to take a bit of a hybrid approach from the above PRs. As nhynes mentioned above, its annoying have to go to a "[a] was renamed from [b]" page whenever [a] is the same as [b], so I'm going to try to remove the rename page for that case. I think it is still valuable when they are different.

• I also need to take gnidan's bug report here and fix it, this PR currently doesn't handle it correctly.

• If possible, it would also be nice to be able to avoid the performance loss that this PR introduces with the export trimming.

[Gerrit0]

It turns out just the renames issue is way trickier than I thought it was. It's fairly easy to rewrite all URLs to go directly to the renamed symbol, but I was trying to avoid this since it seems useful for renames that change the name of a symbol... I'll keep looking at it, but I've already come to loathe how TypeDoc currently handles HTML output. Figuring out how URLs are generated + mapped has taken ~6 hours, and I still don't have the complete picture.

[Gerrit0]

#1157 has been merged, which includes this feature. Thank you for the contribution! It was incredibly helpful in determining how this should be implemented.