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

docs: add documentation generation #857

Closed
wants to merge 2 commits into from

Conversation

joeskeen
Copy link

@joeskeen joeskeen commented Feb 8, 2024

This PR takes all the awesome code documentation available in JSDoc/TSDoc comments throughout the code base and generates a documentation static site from it using the TypeDoc command-line tool.

A few helpful commands have been added to package.json:

  • generate-docs: Runs TypeDoc and outputs the docs in the docs folder (ignored from source control)
  • publish-docs: Generates the docs, goes into the docs folder, and commits/pushes the docs up to the gh-pages branch of the repository (by default, goes to bpmn-io/diagram-js, but you can pass another repository in to publish them elsewhere). To test, I ran this with joeskeen/diagram-js and it generated the GitHub Pages site at https://joeskeen.github.io/diagram-js

Changes to the actual content of the documentation is out of scope for this PR, just trying to publish all existing API documentation.

Closes #78

@CLAassistant
Copy link

CLAassistant commented Feb 8, 2024

CLA assistant check
All committers have signed the CLA.

@nikku
Copy link
Member

nikku commented Feb 8, 2024

Your website rocks, and so do you. Thank you so much!

We'll have a closer look beginning of next week.

@nikku
Copy link
Member

nikku commented Feb 8, 2024

Some things I found having a shallow look:

@joeskeen
Copy link
Author

joeskeen commented Feb 8, 2024

Classes are exposed as default, cf. CommandStack instead of their names.

@nikku I have resolved this with #858

@joeskeen
Copy link
Author

joeskeen commented Feb 8, 2024

Would be great to link this back to source code somehow

@nikku I did look into this, and I'm finding that it's not possible currently with the way things work. From what I understand, you use your library bio-dts to generate the .d.ts files for each of the JS source files. But since the docs created via TypeDoc only work with TypeScript files, and since the .d.ts files are not checked in, there is no way to refer to the source. The TypeDoc tool does create references, but they are to the .d.ts files which don't resolve as links.

image

It doesn't appear that bio-dts creates source maps for the symbols in the generated files, but if it could, we may be able to map those links back to the original source.

Another alternative would be to use jsdoc instead of typedoc. I tried this out and found that the output was really not great compared to what we are getting through typedoc. My guess is that your bio-dts tool does a better job understanding the JS source than jsdoc does, so typedoc has a better input as a result. Thoughts?

Edit: one more option could be to write a TypeDoc plugin that rewrites the source links then have the link originally point to the .d.ts as if it existed on GitHub (possible via the typedoc config):

"sourceLinkTemplate": "https://github.com/bpmn-io/diagram-js/blob/master/{{sourceFile}}#L{{line}}",

I don't see any way to dynamically transform the link (like replacing .d.ts with .js), but a plugin to do that should be pretty easy to whip up.

Edit 2:

I created a TypeDoc plugin that links to the GitHub source JavaScript files

const { Converter, DeclarationReflection, SignatureReflection } = require("typedoc");

module.exports.load = function load(app) {
    app.converter.on(Converter.EVENT_RESOLVE_BEGIN, (context) => {
        if (app.options.disableSources) return;
        
        for (const id in context.project.reflections) {
            const refl = context.project.reflections[id];

            if (
                !(
                    refl instanceof DeclarationReflection ||
                    refl instanceof SignatureReflection
                )
            ) {
                continue;
            }

            for (const source of refl.sources || []) {
                const fileName = source.fileName;
                const jsPath = fileName.replace(/\.d\.ts$/, '.js');
                source.url = `https://github.com/bpmn-io/diagram-js/blob/develop/${jsPath}`;
            }
        }
    });
}

With that the docs say Defined in [lib/core/Canvas.d.ts:10](https://github.com/bpmn-io/diagram-js/blob/develop/lib/core/Canvas.js) but the link points to https://github.com/bpmn-io/diagram-js/blob/develop/lib/core/Canvas.js, which I think is acceptable.

@nikku if you like this approach I can add this plugin to the PR.

@joeskeen
Copy link
Author

joeskeen commented Feb 8, 2024

Some external dependencies are not documented; this will bite us in downstream libraries (i.e. bpmn-js) - https://joe.skeen.rocks/diagram-js/types/Diagram.ModuleDeclaration.html.

@nikku I believe this is because the .d.ts files generated do not have any doc comments on the type aliases from the didi package (or others). I would expect the only way to fix this would be to alter the bio-dts process to add doc comments to those encountered @typedefs. See https://stackoverflow.com/a/62243905

Edit: also looking further into this, I see the Diagram.d.ts file defines ModuleDeclaration thusly:

type ModuleDeclaration = import('didi').ModuleDeclaration;

As this symbol is not exported, it does not get included in the documentation. Having bio-dts export that type would help. TypeDoc actually warns about this:

[warning] ModuleDeclaration, defined in ./lib/Diagram.d.ts, is referenced by Diagram.DiagramOptions.__type.modules but not included in the documentation.

If I manually edit the generated Diagram.d.ts as follows:

/**
 * The [didi](https://github.com/nikku/didi) module declaration type.
 */
export type ModuleDeclaration = import('didi').ModuleDeclaration;

Then the docs show this:
image
... with a link to the didi repository.

@joeskeen
Copy link
Author

joeskeen commented Feb 8, 2024

Links don't seem to resolve consistently (https://joe.skeen.rocks/diagram-js/classes/core_ElementFactory.default.html#create.create-1)

I'm not sure what you mean. In the example you provided above, it seems that the link resolution works as follows:

@nikku
Copy link
Member

nikku commented Feb 15, 2024

Thanks for your follow-ups on this topic. Find some comments attached:

My guess is that your bio-dts tool does a better job understanding the JS source than jsdoc does, so typedoc has a better input as a result. Thoughts?

This is my thinking, too. We could do some magic at some point to find back-references to the source code from type definitions. Not impossible, but also not a priority today. Types are strictly validated and non-ambiguous, hence it makes sense to use them as a baseline.

It doesn't appear that bio-dts creates source maps for the symbols in the generated files, but if it could, we may be able to map those links back to the original source.

We indeed don't add sourcemaps as you indicated, but it should be possible to add them, linking back to the source code from a type declaration file.

I created a typedoc plugin [...]

With that [plug-in] the docs say Defined in lib/core/Canvas.d.ts:10 but the link points to https://github.com/bpmn-io/diagram-js/blob/develop/lib/core/Canvas.js, which I think is acceptable.

I think we could park this item for now, as generating types with sourcemap sounds like the better solution to me.

@nikku
Copy link
Member

nikku commented Feb 15, 2024

Regarding generation of external API references:

ref: As this symbol is not exported, it does not get included in the documentation. Having bio-dts export that type would help.

I'm not sure it would help, as just me getting to the didi source code repository does not give me the assistance that I need. Why I'm especially picky about this is because many folks using our tools (and generating documentation) will face the same issue: You cannot document what is built on top of diagram-js (i.e. bpmn-js) without an easy way to source diagram-js documentation.

@nikku
Copy link
Member

nikku commented Feb 15, 2024

Links don't seem to resolve consistently (https://joe.skeen.rocks/diagram-js/classes/core_ElementFactory.default.html#create.create-1)

I'm not sure what you mean. In the example you provided above, it seems that the link resolution works as follows [...]

Point taken, I think it works as expected.

Unrelated to this the file is another showcase for how we need to export all types declared (and used), or we'll not get proper documentation for them (#857 (comment), #857 (comment)):

image

@nikku
Copy link
Member

nikku commented Feb 15, 2024

To summarize what I learned through your investigations:

@nikku
Copy link
Member

nikku commented Feb 19, 2024

I did some more investigation on typedoc documentation generation yesterday:

we don't offer source maps for type declarations which makes it impossible to map types back to JavaScript sources

This is something we could accomplish, as the bio-dts foundations support it out of the box.

we'd need to #857 (comment) (also utility types) or we'll not get the documentation generation we need

It is actually worse than this. In fact we'd need to export all transitive types, which is completely unreasonable for larger libraries. Typescript itself, when generating types, does not export transitive types either.

Typescript offers a clear contract: Everything that is exposed as public API, and hintable in a code editor shall also be documented. The assumptions that typedoc comes with don't work for us, the plug-ins provided don't seem to work to generate better types, but cause endless duplication in the documentation.

linking properly to external documentation is a topic of further exploration

There exists a plug-in for that, but it only works if external documentation is typedoc, too.

Needs to be further assessed (probably simply build our own docs generator).

@nikku
Copy link
Member

nikku commented Feb 26, 2024

Declaration maps are possible, in theory. In practice it takes some work for sources to be properly mapped through our pipeline.

@nikku
Copy link
Member

nikku commented May 7, 2024

@joeskeen Thanks again for your work on this topic. We'll close this PR for now as it is not ready to be incorporated; we may use it as a future reference once we tackle documentation generation ourselves, probably by writing our own tool.

What we did in the mean time is to invest into advanced typing, cf. bpmn-io/bpmn-js#2153, inside your editor 🤩.

@nikku nikku closed this May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide Documentation
3 participants