feat: Support for export declarations

[Gerrit0]

After examining prior implementations (#646, #801, #1079, #1143), and spending a good bit of time trying to satisfy myself with #801, I decided to just try implementing a new type of declaration that references another declaration.

This is the result of that effort. The diff appears far larger than it really is since it includes updates to the tests (specifically, the rendering tests explode for any small change). I recommend looking at just the TS changes

Try it out:

This PR has been published as typedoc@0.16.0-0, which can be installed with npm i typedoc@next

npm i typedoc@next
npx typedoc --out docs src/index.ts
npx serve docs

Comparisons:

Issue	Pros	Cons
#646	Simple implementation, reflection links lead directly to the referenced reflections	Depends on the conversion order, will fail to create reflections in some cases.
#801	Defers resolution until after conversion, does not suffer from conversion order issues	Introduces performance penalties due to early export trimming
#1079	Simple implementation, copies imported reflections as if they existed where they are re-exported	Bloats output size, renamed symbols are confusing + may break links in doc comments
#1143	Simple change to mark reflections exported with export declarations as exported	Does not handle re-exports, only exporting reflections existing in a file.
This PR	Avoids conversion order + deferred conversion problems, relatively simple implementation	All renamed reflections require 2 clicks to get to the actual reflection in the default theme

I think the two biggest problems with this solution are that:

1. Re-exports with the same name require an extra click. - This could be changed by a theme, and I'd be open to accepting a PR that changes it in the default theme, but I've spent way too long trying to figure out how to do this without rebuilding the output builder entirely...
2. References aren't nicely grouped according to their type. - Again, this could be changed by a theme. The Rust docs (example) do the same thing, so maybe this is okay?

Screenshots

a is re-exported using export * from './mod'

ExportedClassName is exported using export { NotExportedClassName as ExportedClassName }

[Gerrit0]

Tagging some people who've been involved recently in this for feedback: @PissedCapslock @nhynes @alalonde @ibezkrovnyi @gnidan

[TheNeuralBit]

I tested this out on Apache Arrow's JS implementation (we use export extensively) and encountered an error:

❯ npm run doc

> apache-arrow@1.0.0-SNAPSHOT doc /home/bhulette/working_dir/arrow/js
> shx rm -rf ./doc && typedoc --options typedoc.js


Using TypeScript 3.7.4 from /home/bhulette/working_dir/arrow/js/node_modules/typedoc/node_modules/typescript/lib
/home/bhulette/working_dir/arrow/js/node_modules/typedoc/dist/lib/converter/nodes/export.js:77
                    if (original.valueDeclaration.getSourceFile() === specifier.getSourceFile() && !specifier.propertyName) {
                                                  ^

TypeError: Cannot read property 'getSourceFile' of undefined
    at node.exportClause.elements.forEach.specifier (/home/bhulette/working_dir/arrow/js/node_modules/typedoc/dist/lib/converter/nodes/export.js:77:51)
    at Array.forEach (<anonymous>)
    at ExportDeclarationConverter.convert (/home/bhulette/working_dir/arrow/js/node_modules/typedoc/dist/lib/converter/nodes/export.js:71:40)
    at Converter.convertNode (/home/bhulette/working_dir/arrow/js/node_modules/typedoc/dist/lib/converter/converter.js:116:53)
    at statements.forEach (/home/bhulette/working_dir/arrow/js/node_modules/typedoc/dist/lib/converter/nodes/block.js:71:28)
    at Array.forEach (<anonymous>)
    at BlockConverter.convertStatements (/home/bhulette/working_dir/arrow/js/node_modules/typedoc/dist/lib/converter/nodes/block.js:70:24)
    at context.withSourceFile (/home/bhulette/working_dir/arrow/js/node_modules/typedoc/dist/lib/converter/nodes/block.js:54:22)
    at Context.withSourceFile (/home/bhulette/working_dir/arrow/js/node_modules/typedoc/dist/lib/converter/context.js:94:9)
    at BlockConverter.convertSourceFile (/home/bhulette/working_dir/arrow/js/node_modules/typedoc/dist/lib/converter/nodes/block.js:45:17)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! apache-arrow@1.0.0-SNAPSHOT doc: `shx rm -rf ./doc && typedoc --options typedoc.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the apache-arrow@1.0.0-SNAPSHOT doc script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/bhulette/.npm/_logs/2019-12-31T21_38_32_021Z-debug.log

If you're interested it should be possible to repro with:

# git clone git@github.com:apache/arrow.git
# cd arrow/js
# npm i typedoc@next
# npm run doc

Or let me know if there's some additional debug info I can provide

[Gerrit0]

Thanks for the report! I should be able to give that a look later today. That error makes me think that one of TypeScript's "non nullable" properties is actually nullable.

[Gerrit0]

I've spent some time looking into this, fixing the original error wasn't difficult (I still think TS typings are wrong but I don't know how to reproduce besides just pointing the team at your repo), but the generated documentation reveals some more problems.

1. How should rendering references work with --mode file? This is certainly less than ideal. I'd really love to tell people not to use file mode, but modules mode buries documentation in a way that is unfriendly to end users. Once this PR is merged the next thing I want to tackle is library mode, with which I hope to solve these issues... but it will probably take at least a couple weeks to get that working like I want it to.
   
2. What do we do with broken references? Running the arrow docs with the default options lists a ton of broken references (which currently render like this because I forgot to handle this case specifically in the theme), fixing, will publish a fix soon
   
   
   (Also open to better wording for this warning, whenever you use exclude* options this could happen)

---

PS: Specific to Arrow

• Despite using shx for rm, the doc build script doesn't work on a Windows system with cmd (or powershell) set as the shell. (&& doesn't work in Powershell, and cmd tries to run typedoc.js with WScript.exe)

[Gerrit0]

0.16.0-1 is out with a fix to the crash in Apache Arrow, the generated docs unfortunately look horrible... (And also... appear to be missing some items that are shown when generating with module mode...?)

[PissedCapslock]

Sorry for the delay, but only now back at work. I tried the latest version on our project, but it looks like the exported members are still not present in the documentation.

I created a repo to reproduce it:

git clone https://github.com/PissedCapslock/typedoc-experiments.git
cd typedoc-experiments
npm install
npm run-script tsdoc
//now open tsdoc/index.html

The expected output is a Foo module with a Foo class containing a single bar method. However, only the Foo module is visible. The actual class is not contained in the documentation.

If I change the Foo.ts file and use

export class Foo { ...}

instead of

class Foo {...}
export {Foo};

the class does appear in the documentation.

As you'll see in the repo, the "special" thing we do is generate the documentation based on the output of the tsc compiler (using the resulting .d.ts files) and not on the .ts files directly.
If this would be the main cause of our problems, I can see if we can change this. However, a quick experimetn with this test repo showed that the same problem is present when generating the docs from the .ts files directly.

The current workaround I'm using is to set

excludeNotExported: true

[Gerrit0]

Thanks @PissedCapslock! I must have gotten confused when looking through the specs, I could have sworn that was working already. Fixed in 0.16.0-2.

[PissedCapslock]

Thanks Gerrit. Tested that new version on our real project and it looks like it solves the issue. Nice work.

[alalonde]

OK, just took a look and ran our (large!) codebase against it. The first thing I noticed is the set of warnings for erroneous exports. This is quite helpful. The second thing I noticed was that exporting object literals is still broken. Though I'm not sure that this PR intended to address that. #801 had addressed this though. We built and deployed our docs from a fork of #801 here: notice how this interface includes the nested type declarations, but when I generate docs against this PR those declarations are omitted.

As for the multiple-export-link issue, I actually built a plugin on top of #801 that addresses that and adds some additional module organizational tags. I'll rebase that to work with this PR and see how it shakes out.

[TheNeuralBit]

Thanks @Gerrit0!

How should rendering references work with --mode file?

Personally I'm ok if we don't answer that question. I only set our up the Arrow docs to use file mode initially because it seemed to produce slightly more logical results for our crazy structure. With this change I think I will likely switch our build back to --mode modules.

https://github.com/apache/arrow/blob/master/js/src/builder/int.ts#L56

What do we do with broken references?

Maybe it should actually be an error and there should be an "allowBrokenReferences" option to enable the behavior you have now? It seems likely that this is an indication something is wrong with your configuration.

For the arrow build I'm actually not sure why these references are broken. I tried removing all the exclude* options and they are still broken.

[Gerrit0]

@alalonde

The second thing I noticed was that exporting object literals is still broken.

This should be fixed in 0.16.0-3 (just published), its technically unrelated to this PR, but the functionality is improved by it and we certainly don't want it to get lost.

As for the multiple-export-link issue...

This looks awesome!

---

@TheNeuralBit

Personally I'm ok if we don't answer that question.

I think this is the right option. File mode should be used iff you aren't actually using modules. With the library mode (almost the next thing on my todo list now!) I hope to make module mode obsolete.

Maybe it should actually be an error and there should be an "allowBrokenReferences" option to enable the behavior you have now? It seems likely that this is an indication something is wrong with your configuration.

I like this idea in principle but unfortunately it goes against how the rest of TypeDoc works. If we don't have docs for something we just list the name without a link, making a best effort no matter the input... (Maybe this philosophy should be changed, but it will require a fair bit of effort in other places around the code to make it consistent)

For the arrow build I'm actually not sure why these references are broken. I tried removing all the exclude* options and they are still broken.

It's because of your @ignore comment! That comment removes the item from the generated docs, so the reference has nothing to point to.... I suspect this will cause a lot of problems... maybe there's a better solution? Maybe references that don't point to anything should just be removed from the docs?

[alalonde]

Just tested 0.16.0-3 and I can confirm that exporting object literals works again. I also rebased my plugin to work with this and got the tests working. Looks good so far. Thanks for your efforts!

[PissedCapslock]

I must be doing something wrong, as I cannot get the result from the screenshots at the start of this topic.

The "Re-exports Bar" creates no link, so I cannot access the documentation for Bar.
Clicking on the Bar links in the TOC also does not work. It just points to the page I'm already on.

I modified my repo used for my previous comment.
You can reproduce this behavior using:

git clone https://github.com/PissedCapslock/typedoc-experiments.git
cd typedoc-experiments
npm install
npm run-script tsdoc
//now open tsdoc/index.html

Repo contains only a single class:

/**
 * The Foo class
 */
class Foo{
  public get bar(): string {
    return "bar";
  }
}

export {Foo as Bar};

Am I doing something wrong, or did this break in one of the fixes after the initial version ?

[Gerrit0]

You should have received a warning when generating the docs about a broken link.

This is expected since you use excludeNotExported: true and Foo is not exported under that name. If you disable that option or also export Foo it should link as expected.

[Gerrit0]

Note to self - this breaks with TS 3.8 due to microsoft/TypeScript#34903

[Gerrit0]

Just published what I hope is the last pre-release for this, I realized a fix I'd made early on was incorrect, fixed it again, found that it was broken with --mode file, fixed that again...

[alalonde]

Are we TS 3.8 compatible again then ?

I updated my plugin to use 0.16.0-6 and there didn't seem to be any regressions or test failures, so... ship it!

[Gerrit0]

It shouldn't break with 3.8 now that I've looked a bit more into the API changes, but this won't catch namespace exports until it's updated to also handle ts.SyntaxKind.NamespaceExport which isn't available yet.

Once 3.8 releases it should be a fairly simple change to support that :)

[Gerrit0]

I'm working on merging this in now, along with #936 and the fix/options branch that fixes a bunch of issues related to options.

There will be (at least) one more pre-release once this is done because it changes a couple things.

1. Moves CommentPlugin.removeReflections? to ProjectReflection where it belongs.
2. Removes ProjectReflection.symbolMapping in favor of ProjectReflection.getReflectionBySymbolId since only one published plugin uses it, I'll submit a PR to fix that plugin on release. This makes it possible to have significantly better performance when removing reflections with @hidden and friends.
3. @hidden/@ignore/@internal comments on reflections will now also hide any references to that reflection. This improves the generated docs for Arrow JS considerably @TheNeuralBit, thanks for the test case that inspired this change :)

There are still broken references in the Arrow docs because apparently we don't currently support export import ArrowType = Schema_.org.apache.arrow.flatbuf.Type, so I'm going to look at this too and see if it is an easy fix. If not, I'll just open an issue to address it later.