docs: add api generation for components#1767
Conversation
batchu5
requested review from
Adi-204,
Florence-Njeri,
asyncapi-bot-eve,
derberg and
magicmatatjahu
as code owners
|
What reviewer looks at during PR reviewThe following are ideal points maintainers look for during review. Reviewing these points yourself beforehand can help streamline the review process and reduce time to merge.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the
📝 WalkthroughWalkthroughAdds jsdoc2md tooling (script, helper, template) and committed API.md; tightens numerous JSDoc typedefs and examples; converts several default exports to named exports; introduces a few runtime guards/early returns; updates package.json scripts and devDependency. Most edits are documentation, typings, and export adjustments. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (1)
packages/components/src/components/Models.js (1)
22-26: Reconsider the @Class documentation placement.This
@classJSDoc block documentsModelinaGenerator, but this class is not defined in this file—it's an external type imported from@asyncapi/modelina(e.g.,PythonGenerator,JavaGenerator, etc. on lines 3-9).Documenting external classes in your own codebase can be misleading to maintainers and documentation generators. Consider either:
- Removing this block entirely and relying on the library's own documentation
- Using
@typedefto create a type alias that references the external generator classesIf you want to keep type documentation, use a typedef instead:
-/** - * @class ModelinaGenerator - * @classdesc Represents a Modelina generator class. - * @param {object} [options] - Optional generator options. - */ +/** + * @typedef {import('@asyncapi/modelina').PythonGenerator | import('@asyncapi/modelina').JavaGenerator | import('@asyncapi/modelina').TypeScriptGenerator | import('@asyncapi/modelina').CSharpGenerator | import('@asyncapi/modelina').RustGenerator | import('@asyncapi/modelina').JavaScriptGenerator} ModelinaGenerator + * Represents any Modelina generator class instance. + */
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/components/API.md(1 hunks)packages/components/api.hbs(1 hunks)packages/components/package.json(2 hunks)packages/components/src/components/CloseConnection.js(1 hunks)packages/components/src/components/HandleMessage.js(1 hunks)packages/components/src/components/MethodGenerator.js(2 hunks)packages/components/src/components/Models.js(1 hunks)packages/components/src/components/QueryParamsVariables.js(1 hunks)packages/components/src/components/RegisterErrorHandler.js(1 hunks)packages/components/src/components/RegisterMessageHandler.js(1 hunks)packages/components/src/components/SendOperations.js(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-05-12T14:23:48.919Z
Learnt from: derberg
Repo: asyncapi/generator PR: 1551
File: apps/generator/docs/generator-template.md:45-73
Timestamp: 2025-05-12T14:23:48.919Z
Learning: AsyncAPI 3.0.0 specification introduces significant structural changes from 2.x:
1. Operations become top-level elements with action property (send/receive) and references to channels
2. Channels use 'address' for the topic path instead of having nested publish/subscribe operations
3. Messages are defined under a 'messages' container in channels
4. The specification decouples operations, channels and messages to improve reusability
5. Servers can use a 'host' property
Applied to files:
packages/components/src/components/SendOperations.jspackages/components/API.md
📚 Learning: 2025-05-12T14:23:48.919Z
Learnt from: derberg
Repo: asyncapi/generator PR: 1551
File: apps/generator/docs/generator-template.md:45-73
Timestamp: 2025-05-12T14:23:48.919Z
Learning: AsyncAPI 3.0.0 specification introduces significant structural changes from 2.x:
1. Operations become top-level elements with references to channels
2. Channels use 'address' for the topic path instead of having nested publish/subscribe
3. Messages are defined under a 'messages' container in channels
4. Servers can use a 'host' property
Applied to files:
packages/components/API.md
📚 Learning: 2025-05-12T14:57:35.024Z
Learnt from: Adi-204
Repo: asyncapi/generator PR: 1557
File: packages/templates/clients/websocket/javascript/components/SendOperation.js:21-21
Timestamp: 2025-05-12T14:57:35.024Z
Learning: In the AsyncAPI generator templates, particularly in the JavaScript WebSocket client, single quotes are used for string literals inside template-generated code (which is itself inside backticks) to preserve template placeholders like ${variable} as literal text in the generated code output.
Applied to files:
packages/components/API.md
🪛 LanguageTool
packages/components/API.md
[grammar] ~2-~2: Use a hyphen to join words.
Context: ...ection method with optional pre and post execution logic.
Parameters
| Name...
(QB_NEW_EN_HYPHEN)
[grammar] ~54-~54: Use a hyphen to join words.
Context: ...andler method with optional pre and post execution logic.
Parameters
| Name...
(QB_NEW_EN_HYPHEN)
[grammar] ~150-~150: Use a hyphen to join words.
Context: ...ration method with optional pre and post execution logic.
Parameters
| Name...
(QB_NEW_EN_HYPHEN)
[grammar] ~163-~163: Use a hyphen to join words.
Context: ...ration method with optional pre and post execution logic.
Parameters
| Name...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
packages/components/API.md
4-4: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
17-17: Multiple headings with the same content
(MD024, no-duplicate-heading)
18-18: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
25-25: Multiple headings with the same content
(MD024, no-duplicate-heading)
26-26: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
34-34: Multiple headings with the same content
(MD024, no-duplicate-heading)
35-35: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
45-45: Multiple headings with the same content
(MD024, no-duplicate-heading)
46-46: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
55-55: Multiple headings with the same content
(MD024, no-duplicate-heading)
56-56: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
72-72: Multiple headings with the same content
(MD024, no-duplicate-heading)
73-73: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
86-86: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
90-90: Multiple headings with the same content
(MD024, no-duplicate-heading)
91-91: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
102-102: Multiple headings with the same content
(MD024, no-duplicate-heading)
103-103: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
111-111: Multiple headings with the same content
(MD024, no-duplicate-heading)
112-112: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
118-118: Multiple headings with the same content
(MD024, no-duplicate-heading)
119-119: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
125-125: Multiple headings with the same content
(MD024, no-duplicate-heading)
126-126: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
134-134: Multiple headings with the same content
(MD024, no-duplicate-heading)
135-135: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
142-142: Multiple headings with the same content
(MD024, no-duplicate-heading)
143-143: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
151-151: Multiple headings with the same content
(MD024, no-duplicate-heading)
152-152: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
164-164: Multiple headings with the same content
(MD024, no-duplicate-heading)
165-165: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
177-177: Multiple headings with the same content
(MD024, no-duplicate-heading)
178-178: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🔇 Additional comments (4)
packages/components/src/components/Models.js (1)
30-30: LGTM: Improved type reference.Changing from an inline constructor signature to the named
ModelinaGeneratortype improves readability and maintainability.packages/components/src/components/SendOperations.js (1)
9-24: LGTM: Well-structured type definitions.The new
SendOperationMethodsandSendOperationGeneratortype definitions improve code documentation and type clarity. The refactoring from an inline type to named types makes the API more maintainable.packages/components/package.json (2)
13-13: No issues found — jsdoc2md command and paths are correctly configured.The verification confirms:
- Template file
api.hbsexists atpackages/components/api.hbs✓- Glob pattern
./src/**/*.jscorrectly matches all 16 source files ✓- Relative path resolution works as expected from the npm script execution context ✓
43-43: No issues found—version 9.1.3 is current and compatible.jsdoc-to-markdown v9.1.3 requires Node.js >= 12.17.0, and the version was released on October 3, 2025, making it current. The specified version is appropriate for the project.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/components/API.md (1)
1-193: Update JSDoc comments in four component files to correct hyphenation.The auto-generated API.md reflects the current state of JSDoc comments in source files. Verification confirms that grammar issues identified in the previous review have not yet been fixed. Update "pre and post execution" to "pre- and post-execution" in the following files:
packages/components/src/components/CloseConnection.jspackages/components/src/components/HandleMessage.jspackages/components/src/components/RegisterErrorHandler.jspackages/components/src/components/RegisterMessageHandler.jsOnce source JSDoc is corrected, the auto-generated API.md will reflect these changes.
🧹 Nitpick comments (1)
packages/components/package.json (1)
15-19: Consider adding API.md to the published package files.The
filesarray currently excludes the auto-generatedAPI.md. Since this is user-facing API documentation, consider whether it should be included in npm package distributions for end-users to reference.If
API.mdshould be published, add it to thefilesarray:"files": [ "lib/**", "README.md", + "API.md", "LICENSE" ],Alternatively, if
API.mdis only for the repository, ensure it's in.gitignoreand document this decision.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/components/API.md(1 hunks)packages/components/api.hbs(1 hunks)packages/components/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/api.hbs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-04-23T09:18:38.333Z
Learnt from: derberg
Repo: asyncapi/generator PR: 1512
File: packages/templates/clients/websocket/javascript/components/AvailableOperations.js:10-15
Timestamp: 2025-04-23T09:18:38.333Z
Learning: In the asyncapi/generator repository, keys aren't required when mapping over operations array in the AvailableOperations component, contrary to the typical React pattern.
Applied to files:
packages/components/package.json
📚 Learning: 2025-05-12T14:57:35.024Z
Learnt from: Adi-204
Repo: asyncapi/generator PR: 1557
File: packages/templates/clients/websocket/javascript/components/SendOperation.js:21-21
Timestamp: 2025-05-12T14:57:35.024Z
Learning: In the AsyncAPI generator templates, particularly in the JavaScript WebSocket client, single quotes are used for string literals inside template-generated code (which is itself inside backticks) to preserve template placeholders like ${variable} as literal text in the generated code output.
Applied to files:
packages/components/API.md
🪛 LanguageTool
packages/components/API.md
[grammar] ~2-~2: Use a hyphen to join words.
Context: ...ection method with optional pre and post execution logic. ### Parameters | Name |...
(QB_NEW_EN_HYPHEN)
[grammar] ~58-~58: Use a hyphen to join words.
Context: ...andler method with optional pre and post execution logic. ### Parameters | Name |...
(QB_NEW_EN_HYPHEN)
[grammar] ~157-~157: Use a hyphen to join words.
Context: ...ration method with optional pre and post execution logic. ### Parameters | Name |...
(QB_NEW_EN_HYPHEN)
[grammar] ~171-~171: Use a hyphen to join words.
Context: ...ration method with optional pre and post execution logic. ### Parameters | Name |...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
packages/components/API.md
4-4: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
18-18: Multiple headings with the same content
(MD024, no-duplicate-heading)
19-19: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
26-26: Multiple headings with the same content
(MD024, no-duplicate-heading)
27-27: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
33-33: Multiple headings with the same content
(MD024, no-duplicate-heading)
36-36: Multiple headings with the same content
(MD024, no-duplicate-heading)
37-37: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
45-45: Multiple headings with the same content
(MD024, no-duplicate-heading)
48-48: Multiple headings with the same content
(MD024, no-duplicate-heading)
49-49: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
56-56: Multiple headings with the same content
(MD024, no-duplicate-heading)
59-59: Multiple headings with the same content
(MD024, no-duplicate-heading)
60-60: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
70-70: Multiple headings with the same content
(MD024, no-duplicate-heading)
77-77: Multiple headings with the same content
(MD024, no-duplicate-heading)
78-78: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
91-91: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
95-95: Multiple headings with the same content
(MD024, no-duplicate-heading)
96-96: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
105-105: Multiple headings with the same content
(MD024, no-duplicate-heading)
108-108: Multiple headings with the same content
(MD024, no-duplicate-heading)
109-109: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
117-117: Multiple headings with the same content
(MD024, no-duplicate-heading)
118-118: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
124-124: Multiple headings with the same content
(MD024, no-duplicate-heading)
125-125: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
131-131: Multiple headings with the same content
(MD024, no-duplicate-heading)
132-132: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
140-140: Multiple headings with the same content
(MD024, no-duplicate-heading)
141-141: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
146-146: Multiple headings with the same content
(MD024, no-duplicate-heading)
149-149: Multiple headings with the same content
(MD024, no-duplicate-heading)
150-150: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
158-158: Multiple headings with the same content
(MD024, no-duplicate-heading)
159-159: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
169-169: Multiple headings with the same content
(MD024, no-duplicate-heading)
172-172: Multiple headings with the same content
(MD024, no-duplicate-heading)
173-173: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
182-182: Multiple headings with the same content
(MD024, no-duplicate-heading)
186-186: Multiple headings with the same content
(MD024, no-duplicate-heading)
187-187: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🔇 Additional comments (1)
packages/components/package.json (1)
43-43: Dependency version is appropriate.The jsdoc-to-markdown v9.1.3 version with caret constraint
^9.1.3is suitable for this use case, allowing patch and minor updates while maintaining stability.
… acroos platforms
batchu5
force-pushed
the
api-generation-for-components
branch
from
ef164c0 to
c521c81
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/components/package.json (1)
15-19: Consider addingAPI.mdto the published files array.The
API.mdfile is generated by thedocsscript and added to the published package alongsideREADME.mdandLICENSE. Confirm whether it should be included in thefilesarray for distribution to package consumers.If
API.mdis meant to be published, apply this diff:"files": [ "lib/**", "README.md", + "API.md", "LICENSE" ],
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/components/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-23T09:18:38.333Z
Learnt from: derberg
Repo: asyncapi/generator PR: 1512
File: packages/templates/clients/websocket/javascript/components/AvailableOperations.js:10-15
Timestamp: 2025-04-23T09:18:38.333Z
Learning: In the asyncapi/generator repository, keys aren't required when mapping over operations array in the AvailableOperations component, contrary to the typical React pattern.
Applied to files:
packages/components/package.json
🔇 Additional comments (1)
packages/components/package.json (1)
12-13: Cross-platform script fixes and glob quoting are correctly applied.The scripts properly use
&&for sequential execution (addressing the prior Unix-only&issue) and the glob pattern is correctly quoted for cross-platform compatibility. The workflow ensuresdocsruns afterprepublishOnlyto generate API documentation.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/components/package.json:
- Line 13: The "docs" npm script uses the jsdoc2md CLI but the package
jsdoc-to-markdown is not declared in devDependencies; add "jsdoc-to-markdown" to
the package.json devDependencies (or run npm/yarn add --dev jsdoc-to-markdown)
so the jsdoc2md command is available for "docs" and "generate:assets"; update
the devDependencies block to include the exact package name and a compatible
version and run install to ensure the CLI is present.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/components/package.jsonpackages/components/src/components/SendOperations.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/src/components/SendOperations.js
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: asyncapi/generator PR: 0
File: .github/pr-review-checklist.md:0-0
Timestamp: 2026-01-05T09:57:34.800Z
Learning: Update relevant Generator documentation to accurately reflect PR changes introduced
Learnt from: CR
Repo: asyncapi/generator PR: 0
File: .github/pr-review-checklist.md:0-0
Timestamp: 2026-01-05T09:57:34.800Z
Learning: Applies to **/*.{js,ts,jsx,tsx} : Write clear and consistent JSDoc comments for functions, including parameter types, return values, and error conditions
📚 Learning: 2026-01-05T09:57:34.800Z
Learnt from: CR
Repo: asyncapi/generator PR: 0
File: .github/pr-review-checklist.md:0-0
Timestamp: 2026-01-05T09:57:34.800Z
Learning: Update relevant Generator documentation to accurately reflect PR changes introduced
Applied to files:
packages/components/package.json
📚 Learning: 2026-01-05T09:57:34.800Z
Learnt from: CR
Repo: asyncapi/generator PR: 0
File: .github/pr-review-checklist.md:0-0
Timestamp: 2026-01-05T09:57:34.800Z
Learning: Applies to **/*.{js,ts,jsx,tsx} : Write clear and consistent JSDoc comments for functions, including parameter types, return values, and error conditions
Applied to files:
packages/components/package.json
🔇 Additional comments (2)
packages/components/package.json (2)
15-20: LGTM!Including
API.mdin the published files ensures consumers have access to the generated API documentation.
12-12: LGTM!The workflow correctly builds first (
prepublishOnly) before running docs generation, ensuring thelib/directory exists whenjsdoc2mdprocesses./lib/**/*.js.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/components/src/components/OnOpen.js`:
- Around line 8-12: The JSDoc for websocketOnOpenMethod is incorrect: it
currently declares Object.<Language, Function> but the map includes entries like
the java key that are objects of framework-specific functions; update the JSDoc
to accurately reflect the union shape (e.g., a Function or a map of
framework->Function) or add a typedef for the framework map and reference it;
locate websocketOnOpenMethod (and the java entry) in OnOpen.js and change the
comment to a union/typedef type that documents both Function and
Object.<Framework, Function>.
🧹 Nitpick comments (3)
packages/components/src/components/readme/CoreMethods.js (2)
3-6: Minor typedef formatting inconsistency.There's an extra space before the closing brace in the union type.
✏️ Suggested fix
-/** - * `@typedef` {'python' | 'javascript' } Language - * Supported programming languages. - */ +/** + * Supported programming languages. + * `@typedef` {'python' | 'javascript'} Language + */This also moves the description before the
@typedeftag, which is the conventional JSDoc placement for typedef descriptions.
37-39: Consider adding defensive handling for unsupported languages.If
languageis not'python'or'javascript',methodConfig[language]returnsundefined, causing a runtime error on destructuring at line 39. While theLanguagetypedef documents valid inputs, adding a guard could improve robustness.🛡️ Optional defensive check
export function CoreMethods({ language }) { const config = methodConfig[language]; + if (!config) { + throw new Error(`Unsupported language: ${language}. Expected 'python' or 'javascript'.`); + } const { msgHandler, errHandler } = config;This is a defensive coding suggestion for future-proofing; the current implementation works correctly when used as documented.
packages/components/src/components/HandleMessage.js (1)
10-10: Type annotation doesn't match actual data structure.The
pythonentry (lines 13-18) omitsmethodDocsentirely, but the type declares it asmethodDocs: string | undefined(property must exist, value can be undefined). For a property that may be absent, use optional syntax.Suggested fix
-* `@type` {Record<Language, { methodDocs: string | undefined, methodLogic: string }>} +* `@type` {Record<Language, { methodDocs?: string, methodLogic: string }>}
batchu5
force-pushed
the
api-generation-for-components
branch
from
2be5fc0 to
c4583e3
Compare
|
@Adi-204, I have made the changes as per the last conversation. Could you please review once more? |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/components/src/components/OnClose.js (1)
91-96: Guard against undefinedgenerateOnCloseCodeto prevent TypeError.When
language='java'butframeworkis invalid (e.g.,'spring'),websocketOnCloseMethod['java']exists (passes line 91 check), butresolveCloseConfigreturnsundefined. Line 93 then throwsTypeError: generateOnCloseCode is not a function.🐛 Proposed fix
if (websocketOnCloseMethod[language]) { const generateOnCloseCode = resolveCloseConfig(language, framework); - const closeResult = generateOnCloseCode(title); - onCloseMethod = closeResult.onCloseMethod; - indent = closeResult.indent ?? 0; + if (typeof generateOnCloseCode === 'function') { + const closeResult = generateOnCloseCode(title); + onCloseMethod = closeResult.onCloseMethod; + indent = closeResult.indent ?? 0; + } }packages/components/src/components/OnOpen.js (1)
85-94: Throw on unsupported language to match the@throwscontract.
Right now, an unknownlanguagesilently returns an emptyText, which contradicts the JSDoc and can hide misconfigurations. Consider failing fast when the language isn’t supported.🐛 Proposed fix
- if (websocketOnOpenMethod[language]) { - const generateOnOpenCode = resolveOpenConfig(language, framework); - if (typeof generateOnOpenCode !== 'function') { - throw new Error(`Unsupported onOpen handler for language="${language}" framework="${framework}"`); - } - const openResult = generateOnOpenCode(title); - onOpenMethod = openResult.onOpenMethod; - indent = openResult.indent ?? 0; - newLines = openResult.newLines ?? 1; - } + if (!websocketOnOpenMethod[language]) { + throw new Error(`Unsupported onOpen handler for language="${language}" framework="${framework}"`); + } + const generateOnOpenCode = resolveOpenConfig(language, framework); + if (typeof generateOnOpenCode !== 'function') { + throw new Error(`Unsupported onOpen handler for language="${language}" framework="${framework}"`); + } + const openResult = generateOnOpenCode(title); + onOpenMethod = openResult.onOpenMethod; + indent = openResult.indent ?? 0; + newLines = openResult.newLines ?? 1;
🤖 Fix all issues with AI agents
In `@packages/components/src/components/MethodGenerator.js`:
- Around line 19-23: Add explicit `@param` and `@returns` JSDoc tags to the two
private helper functions in MethodGenerator.js (the one documented by the
"Resolve docs and logic for the given language + framework config." block and
the other helper around the 44-48 region). For each helper, document all
parameters by name with a brief type/description and add a `@returns` tag
describing the return value/type; keep the existing `@private` tag and ensure the
JSDoc names match the actual function parameter names so IDEs and linters pick
them up.
In `@packages/components/src/components/readme/CoreMethods.js`:
- Around line 19-35: The JSDoc for the CoreMethods component does not document
that it can throw for unsupported languages; update the comment block for
CoreMethods to include a `@throws` tag describing the error condition (e.g., when
props.language is not recognized) and the type/shape of the thrown error,
referencing the component name CoreMethods and the props.language parameter so
callers know this function may throw for unsupported language values.
🧹 Nitpick comments (1)
packages/components/src/components/OnClose.js (1)
50-58: Add JSDoc forresolveCloseConfighelper function.This function lacks documentation. Per coding guidelines, functions should have clear JSDoc comments including parameter types, return values, and error conditions.
📝 Suggested JSDoc
+/** + * Resolves the appropriate onClose code generator for the given language and optional framework. + * + * `@param` {Language} language - The target programming language. + * `@param` {string} [framework=''] - Optional framework variant (e.g., 'quarkus' for java). + * `@returns` {Function|undefined} The code generator function, or undefined if not found. + */ const resolveCloseConfig = (language, framework = '') => {Based on learnings, JSDoc comments should cover parameter types, return values, and error conditions.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/components/src/components/OnClose.js (1)
58-66: Potential crash when language requires a framework but none is provided.If
languageis'java'andframeworkis empty/undefined,resolveCloseConfigreturnsundefined. Line 101 then callsgenerateOnCloseCode(title)onundefined, causing a runtime error.🐛 Proposed fix: add a guard before invoking the generator
if (websocketOnCloseMethod[language]) { const generateOnCloseCode = resolveCloseConfig(language, framework); + if (!generateOnCloseCode) { + return <Text indent={0}>{''}</Text>; + } const closeResult = generateOnCloseCode(title); onCloseMethod = closeResult.onCloseMethod; indent = closeResult.indent ?? 0; }Alternatively, consider logging a warning or throwing a descriptive error when a required framework is missing for nested configurations.
Also applies to: 99-104
batchu5
force-pushed
the
api-generation-for-components
branch
from
283b318 to
5e4b4d9
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/components/src/components/MethodGenerator.js (1)
10-17: JSDoc type is missingparameterWrapproperty.The type annotation at line 10 doesn't include
parameterWrap, but all runtime configurations (lines 13-16) define this property. This creates a mismatch between the documented type and the actual object shape.Suggested fix
-* `@type` {Record<Language, { returnType: string | undefined, openingTag: string | undefined, closingTag: string | undefined, indentSize: number | undefined }>} +* `@type` {Record<Language, { returnType: string | undefined, openingTag: string | undefined, closingTag: string | undefined, indentSize: number | undefined, parameterWrap: boolean | undefined }>}
🧹 Nitpick comments (2)
packages/components/src/components/MethodGenerator.js (2)
29-48: Return type may not accurately reflect possibleundefinedvalues.The
@returnsannotation declares{ docs: string, logic: string }, but when no config matches andmethodDocs/methodLogicweren't provided, the function returns{ docs: undefined, logic: undefined }(lines 32-33, 48).Suggested fix
-* `@returns` {{ docs: string, logic: string }} +* `@returns` {{ docs: string | undefined, logic: string | undefined }}
91-120: Example doesn't demonstrate the framework-level override path.The example sets
framework = "quarkus"(line 101), butmethodConfigat line 100 doesn't have a nested framework structure. For the framework path (lines 38-41) to be exercised, the config should nest under the framework key:const methodConfig = {"java" : {"quarkus": {methodDocs: methodDocs, methodLogic: methodLogic }}};With the current example, the code falls through to the language-level path (lines 42-44) since
config["quarkus"]is undefined. The example still works but doesn't illustrate the framework feature it appears to demonstrate.Suggested fix to demonstrate framework-level configuration
-* const methodConfig = {"java" : {methodDocs : methodDocs, methodLogic: methodLogic }}; +* const methodConfig = {"java" : {"quarkus": {methodDocs : methodDocs, methodLogic: methodLogic }}};
batchu5
force-pushed
the
api-generation-for-components
branch
from
5e4b4d9 to
cc3ff8d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/components/src/components/MethodGenerator.js (1)
10-17: JSDoc type is missingparameterWrapproperty.The
defaultMethodConfigtype annotation omitsparameterWrap, which is present in all language configurations (lines 13-16). This inconsistency will cause the generated API docs to be incomplete.Suggested fix
-* `@type` {Record<Language, { returnType: string | undefined, openingTag: string | undefined, closingTag: string | undefined, indentSize: number | undefined }>} +* `@type` {Record<Language, { returnType: string | undefined, openingTag: string | undefined, closingTag: string | undefined, indentSize: number | undefined, parameterWrap: boolean | undefined }>}Based on learnings: "Write clear and consistent JSDoc comments for functions, including parameter types, return values, and error conditions."
🤖 Fix all issues with AI agents
In `@packages/components/src/components/OnClose.js`:
- Around line 71-75: Update the JSDoc for the OnClose component to clearly state
that props.framework is required for framework-specific languages (e.g., when
props.language === 'java' the config is nested) and list acceptable framework
values or behavior; additionally, inside the OnClose function add validation
that checks props.language and props.framework (when language === 'java') and
either throw a descriptive error or render a visible warning instead of
producing an empty <Text> when framework is missing/unknown. Reference: OnClose
component, props.framework, props.language, props.title.
🧹 Nitpick comments (1)
packages/components/src/components/MethodGenerator.js (1)
19-49: Return type may not match runtime behavior.The
@returnsannotation specifies{ docs: string, logic: string }, but whenmethodConfigis not provided andmethodDocs/methodLogicare undefined, the function returns{ docs: undefined, logic: undefined }(lines 32-33 assign directly without defaults).Suggested fix
-* `@returns` {{ docs: string, logic: string }} +* `@returns` {{ docs: string | undefined, logic: string | undefined }}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/components/src/components/OnClose.js`:
- Around line 99-103: The OnClose component currently calls
resolveCloseConfig(language, framework) and silently returns an empty <Text>
when generateOnCloseCode is undefined, causing framework-specific languages
(e.g., java) to render blank when Connect renders <OnClose language={language}
title={title} /> without a framework; fix by passing the framework through from
the caller and surfacing a visible warning when framework is missing: update the
Connect.js call site to forward framework into <OnClose language={language}
title={title} framework={framework} /> (or the appropriate prop name you use),
and in OnClose (where websocketOnCloseMethod and resolveCloseConfig are used)
replace the silent return with a clear Text warning message (or throw) that
includes the language name and instructs the caller to provide a framework when
resolveCloseConfig(language, framework) returns undefined.
|
@Adi-204 I fixed the lint errors. Can you please review again? |
|
derberg
left a comment
derberg
left a comment
There was a problem hiding this comment.
please create folloup issue
- Component API.md should be generated in https://github.com/asyncapi/generator/tree/master/apps/generator/docs. As api_components.md
- Make sure front matter is added like in https://github.com/asyncapi/generator/blob/master/apps/generator/docs/api.md?plain=1#L1
title: Reusable Components API
weight: 77
|
/rtm |
4 participants
Description
Adds automated API documentation generation for components using jsdoc-to-markdown and a custom Handlebars template.
This improves consistency and readability of the generated
API.mdfile.Changes
package.jsonscripts:generate:assetsnow runs bothprepublishOnlyanddocsdocsusesjsdoc2mdwith a customapi.hbstemplate to generateAPI.mdsrc/**/*.jsinto a clean Markdown API fileRelated issue(s)
Resolves #1656
Summary by CodeRabbit
New Features
Documentation
Refactor
Behavior
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.