feat!: JSON doc generation #533

Chriscbr · 2021-12-24T00:32:55Z

Supersedes #481
Part of cdklabs/construct-hub-webapp#714 and cdklabs/construct-hub#142

Allows rendering language specific documentation to a JSON object in addition to a markdown string. This allows the consumer more flexibility to render the structure of the API reference as needed without regenerating markdown documents.

This PR builds upon #481 by cleaning up the JSON schema (in schema.ts) and also changing the markdown rendering logic to be based on the JSON output -- so the markdown rendering does not depend on any TranspiledXxx classes or interfaces.

All headers now have anchor tags which default to the unique language-agnostic FQNs (e.g. @aws-cdk/aws-s3.Bucket.parameter.scope). Based on my research, all characters used in JSII FQNs (A-Za-z0-9_-/.@) are allowed as URL fragments, and are also allowed as id's in HTML5, so this is a reasonable default.

This PR also extends the notion of fully qualified names to all "entities" in the document (methods, properties, enum members, and even method parameters) denoted by the "id" fields within the JSON schema, so that any section of a document can be uniquely linked - not just types. The JsiiEntity interface in schema.ts is intended to encapsulate any information that should be needed to render a link, including the package version it's from (this is necessary for Construct Hub to be able to link to the correct versions of packages).

Markdown generation can now be customized using three hooks:

anchorFormatter: (type: JsiiEntity) => string - customize the IDs given to anchors tags that are placed next to every header in the document, so that any part of the API reference can be directly linked
linkFormatter: (type: JsiiEntity, metadata: AssemblyMetadataSchema) => string - customize how links should be rendered when they appear in tables, descriptions, etc. -- e.g. to include logic to "link to another page"
typeFormatter?: (type: TypeSchema, linkFormatter) => string - customize how composite types should be rendered when they include nested types - this allows functionality like the screenshot below:

See markdown.test.ts for an example of how the markdown rendering hooks would be used to format links for Construct Hub.

Examples:

@aws-cdk/aws-ecr Python API.json: https://gist.github.com/Chriscbr/731b315808faaf2a2161d686ce248368
@aws-cdk/aws-ecr Python API.md: https://gist.github.com/Chriscbr/d0833a9ecce258dead648a63253b8b0e

BREAKING CHANGE: Documentation.render is now named Documentation.toMarkdown. In addition, the options parameter for this method has changed. TranspiledType is no longer exported.

Chriscbr · 2021-12-24T00:36:48Z

(moved comments to PR description)

src/docgen/render/markdown.ts

test/docgen/view/parameter.test.ts

Chriscbr · 2022-01-02T21:26:20Z

test/docgen/view/documentation.test.ts

@@ -33,16 +33,6 @@ describe('extractPackageName', () => {

 });

-test('custom link formatter', async () => {


Moved this test to markdown.test.ts

Chriscbr · 2022-01-02T21:34:17Z

src/docgen/util.ts

+    remarks: docs.remarks.length > 0 ? docs.remarks : undefined,
+    see: docs.link.length > 0 ? docs.link : undefined,
+    deprecated: docs.deprecated === true ? true : undefined,
+    deprecationReason: docs.deprecationReason,


We can add more fields from this at any later point as needed (e.g. stability, examples, or other custom @ tags)

Chriscbr · 2022-01-02T21:34:52Z

src/docgen/transpile/python.ts

-    parentType: property.parentType,
+    parentType: callable.parentType,


small bug fix needed to get links working correctly

rix0rrr · 2022-01-04T09:58:13Z

One opinionated change I made was to change how code is rendered by default, from the usual markdown syntax to using HTML pre (multi-line) and code (single-line) tags. Doing this allows us to include links within snippets even when they're rendered on GitHub, as shown above.

I want to say: what's the point of MarkDown anymore at this stage 😅 but I support the usability improvements

rix0rrr

I like what you're doing, and I appreciate the effort. I'd prefer to see the MarkDown renderer factored out into its own visitor class. Seems like a neater separation of concerns, makes it easier to add new types of renders, and more in line with the final processing pipeline.

rix0rrr · 2022-01-04T12:29:17Z

src/docgen/render/markdown.ts

+
+export const defaultLinkFormatter = (type: JsiiEntity) => {
+  const name = type.fqn.split('.').pop()!;
+  return `<a href="#${type.id}">${name}</a>`;


What if the type comes from a different assembly?

In that case we just shrug it off. This is just a default after all. The id is unique across assemblies, so in the worst case the link is invalid. You can see this in the example gist here, where if you click on a type named Construct, the link doesn't take you anywhere.

I think this is good enough for the default.

Actually, I suppose we could display no link if the type isn't from the current assembly. I'll leave this as a future enhancement for now since it's not too critical

See I disagree with this.

In terms of user experience, a link that doesn't work seems worse than no link at all. How annoyed would you be if you clicked something that's obviously a link and nothing happened? You'd click a couple times more, and then start to distrust every other link you'd see.

I'd rather have that be in the initial implementation. Doesn't seem like a huge implementation stretch, but correct me if I'm wrong.

Got it, I see your point now - yeah dead links are a pretty bad experience. 😅

I've added another parameter to the linkFormatter API to enable this behavior -- as of right now, linkFormatter is expected to be a pure function, so the formatter needs extra info to make the determination of whether to create a link.

The formatting APIs are getting a bit heavy/awkward so I've added some more docs and marked them experimental for now. It might make sense eventually just require anyone that wants to customize the markdown linking and formatting etc. to just implement their own markdown renderer at the end of the day -- but for now I'd rather keep these (awkward) APIs just to avoid duplicating code between here and Construct Hub. But any/all suggestions are still welcome. 🙂

src/docgen/render/markdown.ts

src/docgen/schema.ts

src/docgen/view/class.ts

Chriscbr · 2022-01-10T16:32:35Z

@rix0rrr I've given a shot at refactoring all of the markdown-ification into a visitor class - definitely feels like an improvement. Let me know what you think.

rix0rrr

I think I'd like to see the links changed. Let's not generate links that don't go anywhere.

Other than that, <3 this and ready to ship it!

README.md

Chriscbr · 2022-01-12T20:47:57Z

I'm getting timed out trying to create or update gists on GitHub - but here's a screenshot of the "fixed" links - for types that are not in the current document, we just show the full language-specific fqn.

iliapolo

Looks great!

src/docgen/render/markdown-doc.ts

src/docgen/render/markdown-render.ts

src/docgen/view/documentation.ts

src/docgen/view/property.ts

test/docgen/view/__snapshots__/class.test.ts.snap

src/docgen/schema.ts

Signed-off-by: github-actions <[email protected]>

Chriscbr · 2022-01-15T01:12:10Z

test/docgen/view/__snapshots__/documentation.test.ts.snap

-                    "packageVersion": "3.3.187",
+                    "packageVersion": "3.3.188",


Hrmmmmm. It looks like what's happening is that since most packages we generate have caret dependencies, the version of the referenced type isn't necessarily fixed.

I've been thinking about this for a while, including whether there's some better way to model this so that the package version stays fixed given the current assembly (e.g. by including the semver string from package.json instead) -- but it turns out that forces a lot of additional complexity onto downstream users.

For now I'm going to simply add some extra code that will make the snapshots stable by always linking to the same package version within unit tests (e.g. "1.2.3")

iliapolo · 2022-01-16T12:25:24Z

src/docgen/transpile/transpile.ts

+      // may belong to dependency packages, which could be specified with caret
+      // dependencies - this means the packageVersion of a type like
+      // `construct.Construct` will be set to whichever version is installed.
+      // This is okay in practice, but makes snapshot tests inconsistent.


~~Why is this a problem though? Our snapshots will get updated during self-mutation~~

Scratch that, its inconsistent over time you mean, unrelated to dependency upgrades 👍

feat: JSON doc generation

5b4761b

Chriscbr commented Dec 24, 2021

View reviewed changes

src/docgen/render/markdown.ts Outdated Show resolved Hide resolved

Chriscbr and others added 7 commits December 29, 2021 13:58

minor schema cleanup

561fd3d

add anchor config

f651d1a

minor cleanup

ee829e1

add anchor customization, and other fixes

346fac7

update README, unit tests

af106ef

chore: self mutation

ad39e3a

improve docstrings

969a83d

Chriscbr commented Jan 2, 2022

View reviewed changes

Chriscbr marked this pull request as ready for review January 2, 2022 23:42

Chriscbr requested review from iliapolo, rix0rrr and a team January 2, 2022 23:43

rix0rrr requested changes Jan 4, 2022

View reviewed changes

Chriscbr added 2 commits January 7, 2022 11:50

fixes for compatibility with Construct Hub

e66c1e7

PR feedback

ef8d57b

Chriscbr force-pushed the rybickic/json branch from a6377c3 to ef8d57b Compare January 7, 2022 21:32

Chriscbr and others added 7 commits January 7, 2022 13:43

Markdown -> MarkdownDocument

c800b43

refactor to use visitor pattern

4e529ec

fixes

eed0d13

minor refactor

7f3c8e6

update snapshots

a504f04

Merge branch 'main' into rybickic/json

2a24ab3

chore: self mutation

5befa08

Chriscbr requested a review from rix0rrr January 10, 2022 16:29

Chriscbr changed the title ~~feat: JSON doc generation~~ feat!: JSON doc generation Jan 10, 2022

rix0rrr changed the title ~~feat!: JSON doc generation~~ feat: JSON doc generation Jan 12, 2022

rix0rrr requested changes Jan 12, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

Chriscbr added 2 commits January 12, 2022 12:31

PR feedback

12a1e39

Merge branch 'main' into rybickic/json

5c72af2

Chriscbr requested a review from rix0rrr January 12, 2022 20:48

fix anchor names

8328ac7

iliapolo requested changes Jan 13, 2022

View reviewed changes

PR feedback

fe86ccf

Chriscbr changed the title ~~feat: JSON doc generation~~ feat!: JSON doc generation Jan 13, 2022

Chriscbr and others added 3 commits January 13, 2022 15:08

fix parameter ids

6359423

sanitize spaces from IDs

868ee1f

chore: self mutation

3ffefd3

Signed-off-by: github-actions <[email protected]>

Chriscbr requested a review from iliapolo January 14, 2022 06:51

Chriscbr commented Jan 15, 2022

View reviewed changes

make snapshots stable

eb274c9

iliapolo reviewed Jan 16, 2022

View reviewed changes

minor consistency fix

2673327

rix0rrr approved these changes Jan 18, 2022

View reviewed changes

iliapolo approved these changes Jan 18, 2022

View reviewed changes

Merge branch 'main' into rybickic/json

0f64563

mergify bot merged commit 757aa06 into main Jan 18, 2022

mergify bot deleted the rybickic/json branch January 18, 2022 22:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: JSON doc generation #533

feat!: JSON doc generation #533

Chriscbr commented Dec 24, 2021 •

edited

Loading

Chriscbr commented Dec 24, 2021 •

edited

Loading

Chriscbr Jan 2, 2022

Chriscbr Jan 2, 2022

Chriscbr Jan 2, 2022

rix0rrr commented Jan 4, 2022

rix0rrr left a comment

rix0rrr Jan 4, 2022

Chriscbr Jan 7, 2022

Chriscbr Jan 7, 2022

rix0rrr Jan 12, 2022 •

edited

Loading

Chriscbr Jan 12, 2022 •

edited

Loading

Chriscbr commented Jan 10, 2022

rix0rrr left a comment

Chriscbr commented Jan 12, 2022

iliapolo left a comment

Chriscbr Jan 15, 2022

iliapolo Jan 16, 2022 •

edited

Loading

iliapolo Jan 16, 2022

		@@ -33,16 +33,6 @@ describe('extractPackageName', () => {

		});

		test('custom link formatter', async () => {

		parentType: property.parentType,
		parentType: callable.parentType,

feat!: JSON doc generation #533

feat!: JSON doc generation #533

Conversation

Chriscbr commented Dec 24, 2021 • edited Loading

Chriscbr commented Dec 24, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rix0rrr commented Jan 4, 2022

rix0rrr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rix0rrr Jan 12, 2022 • edited Loading

Choose a reason for hiding this comment

Chriscbr Jan 12, 2022 • edited Loading

Choose a reason for hiding this comment

Chriscbr commented Jan 10, 2022

rix0rrr left a comment

Choose a reason for hiding this comment

Chriscbr commented Jan 12, 2022

iliapolo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iliapolo Jan 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chriscbr commented Dec 24, 2021 •

edited

Loading

Chriscbr commented Dec 24, 2021 •

edited

Loading

rix0rrr Jan 12, 2022 •

edited

Loading

Chriscbr Jan 12, 2022 •

edited

Loading

iliapolo Jan 16, 2022 •

edited

Loading