fix: variadic arguments are incorrectly displayed as base type in docs #1556

sumupitchayan · 2024-09-03T19:07:41Z

Fixes #648

Fixes issue where variadic arguments are incorrectly displayed as the base type in documentation.

Example from the original issue:

public add(...scopes: IDependable[])

is shown as:

public void add(IDependable scopes)

…assemblies class Signed-off-by: Sumu <sumughan@amazon.com>

…rkdown renderer Signed-off-by: Sumu <sumughan@amazon.com>

Signed-off-by: Sumu <sumughan@amazon.com>

src/docgen/render/markdown-render.ts

Signed-off-by: Sumu <sumughan@amazon.com>

iliapolo

Couple high level comments:

We talked about fixing the assembly.system.classes to use assembly.classes in the various lookup functions. Are you planning on doing this in a separate PR?
PR title should indicate what the specific problem was. Remember that it is the first thing customers see in the change log, so it should be as specific as possible. How exactly were variadics wrong in the docs?

src/docgen/render/markdown-render.ts

test/docgen/assemblies.ts

test/docgen/view/initializer.test.ts

src/docgen/render/markdown-render.ts

Co-authored-by: Eli Polonsky <epolon@amazon.com>

sumupitchayan · 2024-09-04T15:02:31Z

We talked about fixing the assembly.system.classes to use assembly.classes in the various lookup functions. Are you planning on doing this in a separate PR?

Yea, I think it'd be better to separate that from this fix. This PR would be cluttered with the snapshot updates from both fixes

Signed-off-by: Sumu <sumughan@amazon.com>

…(for now) Signed-off-by: Sumu <sumughan@amazon.com>

Signed-off-by: Sumu <sumughan@amazon.com>

…language's transpiler Signed-off-by: Sumu <sumughan@amazon.com>

Signed-off-by: Sumu <sumughan@amazon.com>

…dklabs/jsii-docgen into sumughan/fix-variadic-arg-docs

src/docgen/view/parameter.ts

src/docgen/transpile/python.ts

src/docgen/transpile/typescript.ts

test/docgen/assemblies.ts

test/docgen/view/parameter.test.ts

src/docgen/transpile/python.ts

…languages Signed-off-by: Sumu <sumughan@amazon.com>

Signed-off-by: Sumu <sumughan@amazon.com>

Co-authored-by: Eli Polonsky <epolon@amazon.com>

…ces, since properties cannnot be variadic Signed-off-by: Sumu <sumughan@amazon.com>

test/docgen/view/parameter.test.ts

src/docgen/transpile/csharp.ts

src/docgen/transpile/typescript.ts

src/docgen/view/parameter.ts

iliapolo · 2024-09-08T07:15:45Z

src/docgen/view/parameter.ts

+    let typeschema = this.transpiledParam.typeReference.toJson();
+    if (this.parameter.spec.variadic) {
+      typeschema = {
+        formattingPattern: this.transpile.variadicOf(this.transpiledParam.typeReference.toString()),


This is tricky to understand. Please explain with some examples.

src/docgen/transpile/csharp.ts

iliapolo · 2024-09-08T07:47:49Z

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


 ---

 ##### \`deps\`<sup>Required</sup> <a name="deps" id="DependencyGroup.Initializer.parameter.deps"></a>

- *Type:* <a href="/packages/constructs/v/99.99.99/api/IDependable?lang=python">IDependable</a>


Hmm, this doesn't look right...somehow we are dropping the link. This should be:

...<a href="/packages/constructs/v/99.99.99/api/IDependable?lang=python">IDependable</a>

Co-authored-by: Eli Polonsky <epolon@amazon.com>

Signed-off-by: github-actions <github-actions@github.com>

Co-authored-by: Eli Polonsky <epolon@amazon.com>

…t function Signed-off-by: Sumu <sumughan@amazon.com>

…dklabs/jsii-docgen into sumughan/fix-variadic-arg-docs

…parameter.ts Signed-off-by: Sumu <sumughan@amazon.com>

Signed-off-by: Sumu <sumughan@amazon.com>

src/docgen/transpile/csharp.ts

iliapolo · 2024-09-10T05:13:15Z

@sumupitchayan Any reason this PR is still in draft?

sumupitchayan · 2024-09-10T13:03:45Z

@sumupitchayan Any reason this PR is still in draft?

@iliapolo nope, just marked as ready

sumupitchayan added 3 commits September 3, 2024 14:58

initial WIP - set up tests, add variadic assembly and helper func to …

a50bd46

…assemblies class Signed-off-by: Sumu <sumughan@amazon.com>

Add variadic field to Parameter schema, parameter constructor, and ma…

f3cd642

…rkdown renderer Signed-off-by: Sumu <sumughan@amazon.com>

update snapshots with variadic flag

f9b3f4b

Signed-off-by: Sumu <sumughan@amazon.com>

sumupitchayan commented Sep 3, 2024

View reviewed changes

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

add instance-method test and create snapshots for it

9596cc1

Signed-off-by: Sumu <sumughan@amazon.com>

iliapolo requested changes Sep 4, 2024

View reviewed changes

sumupitchayan and others added 2 commits September 4, 2024 08:45

Update test/docgen/assemblies.ts

e7a77da

Co-authored-by: Eli Polonsky <epolon@amazon.com>

Update test/docgen/view/initializer.test.ts

3a52b97

Co-authored-by: Eli Polonsky <epolon@amazon.com>

fix build, rename new assembly withVariadicParameter

f72f1ee

Signed-off-by: Sumu <sumughan@amazon.com>

sumupitchayan changed the title ~~fix: variadic arguments in docs~~ fix: variadic arguments are displayed as base type in docs Sep 4, 2024

sumupitchayan changed the title ~~fix: variadic arguments are displayed as base type in docs~~ fix: variadic arguments are incorrectly displayed as base type in docs Sep 4, 2024

sumupitchayan and others added 13 commits September 4, 2024 12:24

eli comment - fixup findVariadicInitializer method

ec5f62f

Signed-off-by: Sumu <sumughan@amazon.com>

eli comment: delete unnecessary initializer test

9acb0cb

Signed-off-by: Sumu <sumughan@amazon.com>

remove unnecessary assertion in initializer snapshot test

4b90a7a

Signed-off-by: Sumu <sumughan@amazon.com>

add static method to variadic assembly fixture

524b0a3

Signed-off-by: Sumu <sumughan@amazon.com>

refactor: move all tests to parameter.test.ts

dcf6e58

Signed-off-by: Sumu <sumughan@amazon.com>

delete old initializer test and snapshots

da2b384

Signed-off-by: Sumu <sumughan@amazon.com>

reset snapshots to what they were before - remove 'variadic' keyword …

59a2916

…(for now) Signed-off-by: Sumu <sumughan@amazon.com>

undo changes to markdown-render.ts

dec0968

Signed-off-by: Sumu <sumughan@amazon.com>

add condtitional variadic syntax to formatParameter function of each …

0c63eb8

…language's transpiler Signed-off-by: Sumu <sumughan@amazon.com>

update snapshots

9a6a601

Signed-off-by: Sumu <sumughan@amazon.com>

Merge branch 'main' into sumughan/fix-variadic-arg-docs

f31420b

remove unused function from markdown-render.ts

14c80d0

Signed-off-by: Sumu <sumughan@amazon.com>

Merge branch 'sumughan/fix-variadic-arg-docs' of https://github.com/c…

9ef80a7

…dklabs/jsii-docgen into sumughan/fix-variadic-arg-docs

iliapolo requested changes Sep 5, 2024

View reviewed changes

iliapolo reviewed Sep 5, 2024

View reviewed changes

src/docgen/transpile/python.ts Outdated Show resolved Hide resolved

sumupitchayan and others added 3 commits September 5, 2024 15:26

add variadicOf function to Transpile interface, implement across all …

a7dd9af

…languages Signed-off-by: Sumu <sumughan@amazon.com>

update snapshots

2fa702d

Signed-off-by: Sumu <sumughan@amazon.com>

Update test/docgen/view/parameter.test.ts

6f00baf

Co-authored-by: Eli Polonsky <epolon@amazon.com>

sumupitchayan mentioned this pull request Sep 6, 2024

chore: use assembly.classes instead of assembly.system.classes in test helper functions #1560

Closed

sumupitchayan and others added 2 commits September 6, 2024 14:47

refactor: separate TranspiledProperty and TranspiledParameter interfa…

8773a8b

…ces, since properties cannnot be variadic Signed-off-by: Sumu <sumughan@amazon.com>

Merge branch 'main' into sumughan/fix-variadic-arg-docs

4e588eb

iliapolo reviewed Sep 8, 2024

View reviewed changes

sumupitchayan and others added 14 commits September 9, 2024 09:33

Update test/docgen/view/parameter.test.ts

2cf398d

Co-authored-by: Eli Polonsky <epolon@amazon.com>

Update test/docgen/view/parameter.test.ts

97abe9e

Co-authored-by: Eli Polonsky <epolon@amazon.com>

Update test/docgen/view/parameter.test.ts

38adbb1

Co-authored-by: Eli Polonsky <epolon@amazon.com>

Update src/docgen/transpile/typescript.ts

5ceef1e

Co-authored-by: Eli Polonsky <epolon@amazon.com>

Update src/docgen/transpile/csharp.ts

68cf1d2

Co-authored-by: Eli Polonsky <epolon@amazon.com>

chore: self mutation

7de541e

Signed-off-by: github-actions <github-actions@github.com>

chore: self mutation

ec84de4

Signed-off-by: github-actions <github-actions@github.com>

Update src/docgen/view/parameter.ts

5a48fb9

Co-authored-by: Eli Polonsky <epolon@amazon.com>

eli comment: call this.property instead of this.paramete inside struc…

b405b13

…t function Signed-off-by: Sumu <sumughan@amazon.com>

Merge branch 'sumughan/fix-variadic-arg-docs' of https://github.com/c…

589ac12

…dklabs/jsii-docgen into sumughan/fix-variadic-arg-docs

Eli comment: add comment explaining typeschema/formatting pattern in …

26d69a9

…parameter.ts Signed-off-by: Sumu <sumughan@amazon.com>

update formatting pattern inside variadic type schema

777af88

Signed-off-by: Sumu <sumughan@amazon.com>

update link snapshots

6b977d3

Signed-off-by: Sumu <sumughan@amazon.com>

Merge branch 'main' into sumughan/fix-variadic-arg-docs

685e4dc

iliapolo reviewed Sep 10, 2024

View reviewed changes

src/docgen/transpile/csharp.ts Outdated Show resolved Hide resolved

iliapolo added 2 commits September 10, 2024 08:03

Update src/docgen/transpile/csharp.ts

5430e96

Merge branch 'main' into sumughan/fix-variadic-arg-docs

e081cf9

iliapolo reviewed Sep 10, 2024

View reviewed changes

src/docgen/transpile/csharp.ts Outdated Show resolved Hide resolved

Update src/docgen/transpile/csharp.ts

7eca5b8

iliapolo approved these changes Sep 10, 2024

View reviewed changes

sumupitchayan marked this pull request as ready for review September 10, 2024 13:03

mergify bot merged commit f302233 into main Sep 10, 2024
18 checks passed

mergify bot deleted the sumughan/fix-variadic-arg-docs branch September 10, 2024 13:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: variadic arguments are incorrectly displayed as base type in docs #1556

fix: variadic arguments are incorrectly displayed as base type in docs #1556

sumupitchayan commented Sep 3, 2024 •

edited

Loading

iliapolo left a comment

sumupitchayan commented Sep 4, 2024

iliapolo Sep 8, 2024

iliapolo Sep 8, 2024

iliapolo commented Sep 10, 2024

sumupitchayan commented Sep 10, 2024

fix: variadic arguments are incorrectly displayed as base type in docs #1556

fix: variadic arguments are incorrectly displayed as base type in docs #1556

Conversation

sumupitchayan commented Sep 3, 2024 • edited Loading

iliapolo left a comment

Choose a reason for hiding this comment

sumupitchayan commented Sep 4, 2024

iliapolo Sep 8, 2024

Choose a reason for hiding this comment

iliapolo Sep 8, 2024

Choose a reason for hiding this comment

iliapolo commented Sep 10, 2024

sumupitchayan commented Sep 10, 2024

sumupitchayan commented Sep 3, 2024 •

edited

Loading