refactor(readme): remove language-specific hardcoding from README generation #1790
Conversation
lightning-sagar
requested review from
Adi-204,
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.
|
📝 WalkthroughWalkthroughInstallation now accepts a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
lightning-sagar
changed the title
Adi-204
left a comment
Adi-204
left a comment
There was a problem hiding this comment.
@lightning-sagar left few comments!
packages/components/src/components/readme/AvailableOperations.js
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/components/src/components/readme/MessageExamples.js (2)
27-49: Consider adding language prop for consistency with Installation component.Unlike
Installation.js(which accepts alanguageprop to render language-specific commands), this component renders examples for all languages unconditionally.For consistency and to support language-aware rendering across README components, consider accepting a
languageprop to filter which examples to display.🔎 Example implementation with language prop
-export default function MessageExamples({ operation }) { +export default function MessageExamples({ operation, language }) { const operationId = operation.id(); const messages = getOperationMessages(operation) || []; const messageExamples = []; messages.forEach((message) => { const examples = getMessageExamples(message) || []; examples.forEach((example) => { const payload = example.payload(); - Object.values(languageConfig).forEach(({ render }) => { - messageExamples.push(render(operationId, payload)); - }); + const languages = language ? [language] : Object.keys(languageConfig); + languages.forEach((lang) => { + const config = languageConfig[lang]; + if (config) { + messageExamples.push(config.render(operationId, payload)); + } + }); }); });
27-27: Add JSDoc documentation for the component.The component lacks JSDoc comments explaining its purpose, parameters, and return behavior.
🔎 Example JSDoc addition
+/** + * Renders message examples for an AsyncAPI operation in multiple programming languages. + * + * @param {Object} props - Component props + * @param {Object} props.operation - AsyncAPI operation object with id() method + * @param {string} [props.language] - Optional language filter (e.g., 'javascript', 'python') + * @returns {React.ReactElement|null} Formatted examples or null if no examples exist + */ export default function MessageExamples({ operation }) {Based on coding guidelines from the PR objectives.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/components/test/components/__snapshots__/AvailableOperations.test.js.snapis excluded by!**/*.snappackages/components/test/components/__snapshots__/MessageExamples.test.js.snapis excluded by!**/*.snappackages/templates/clients/websocket/test/integration-test/__snapshots__/integration.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
packages/components/src/components/readme/Installation.jspackages/components/src/components/readme/MessageExamples.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/src/components/readme/Installation.js
🧰 Additional context used
🧬 Code graph analysis (1)
packages/components/src/components/readme/MessageExamples.js (2)
packages/components/src/components/readme/OperationHeader.js (1)
operationId(4-4)apps/react-sdk/src/renderer/renderer.ts (1)
render(63-77)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Acceptance tests for generated templates
- GitHub Check: Test generator as dependency with Node 24
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - macos-latest
- GitHub Check: Test NodeJS PR - ubuntu-latest
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/components/src/components/readme/MessageExamples.js (1)
6-7: Unused properties flagged in previous review.The
labelandcodeBlockproperties remain unused, as noted in the previous review comment. Consider addressing that feedback by either removing these properties or documenting their intended use.Also applies to: 16-17
🧹 Nitpick comments (1)
packages/components/src/components/readme/MessageExamples.js (1)
44-44: Consider converting operationId to snake_case for Python conventions.JavaScript operationIds typically use camelCase (e.g.,
sendMessage), but Python conventions favor snake_case function names (e.g.,send_message).While not a correctness issue, converting the identifier would make examples more idiomatic for Python developers.
🔎 Suggested conversion helper
Add a utility function:
function toSnakeCase(str) { return str.replace(/[A-Z]/g, letter => `_${letter.toLowerCase()}`); }Then update line 44:
-client.${operationId}(${pythonPayload}) +client.${toSnakeCase(operationId)}(${pythonPayload})
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/components/test/components/__snapshots__/AvailableOperations.test.js.snapis excluded by!**/*.snappackages/components/test/components/__snapshots__/MessageExamples.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (1)
packages/components/src/components/readme/MessageExamples.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test generator as dependency with Node 24
- GitHub Check: Acceptance tests for generated templates
- GitHub Check: Test NodeJS PR - macos-latest
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - ubuntu-latest
Adi-204
left a comment
Adi-204
left a comment
There was a problem hiding this comment.
@lightning-sagar left few comments!
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @packages/components/src/components/readme/MessageExamples.js:
- Line 1: The import list is missing a space after the comma between
getOperationMessages and toSnakeCase; update the import statement that currently
imports getMessageExamples, getOperationMessages,toSnakeCase to include a space
so it reads getMessageExamples, getOperationMessages, toSnakeCase (ensure
spacing matches other imports and preserve the same named imports ordering).
- Around line 15-27: Add a JSDoc block above the renderExample function that
documents its purpose (renders a language-specific example snippet), each
parameter (the first param is an object with label: string and codeBlock:
string, plus operationId: string and payload: any), and the return value
(string). Reference the renderExample function and mention it uses toSnakeCase
when codeBlock === 'python' to transform operationId; include types and a short
description for each parameter and the returned template string.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/components/test/components/__snapshots__/AvailableOperations.test.js.snapis excluded by!**/*.snappackages/components/test/components/__snapshots__/MessageExamples.test.js.snapis excluded by!**/*.snappackages/templates/clients/websocket/test/integration-test/__snapshots__/integration.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (1)
packages/components/src/components/readme/MessageExamples.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.github/pr-review-checklist.md)
Write clear and consistent JSDoc comments for functions, including parameter types, return values, and error conditions
Files:
packages/components/src/components/readme/MessageExamples.js
🧠 Learnings (2)
📓 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: Use a concise PR title that follows Conventional Commits guidelines and uses imperative mood (e.g., 'add new helper for listing operations'). Prepend `feat:` or `fix:` only when PATCH/MINOR release must be triggered
📚 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/src/components/readme/MessageExamples.js
🔇 Additional comments (1)
packages/components/src/components/readme/MessageExamples.js (1)
4-13: Excellent language-aware design!The introduction of
languageConfigand the loop-based rendering successfully removes language-specific hardcoding. This design:
- Centralizes language-specific configuration
- Makes adding new languages straightforward (just extend
languageConfig)- Generates examples for all configured languages automatically
This aligns perfectly with the PR objective to make README components language-aware.
Also applies to: 38-40
|
@lightning-sagar Did you go through both CodeRabbit comments carefully? |
sorry about the confusion, I do read both coderabbit comments...,. but I confused, what you meant by “this is valid” that is why I asked for confirmation.... sorry about that |
|
@lightning-sagar oh ok I just mean that comment from coderabbit is valid and please do the changes suggested by coderabbit |
|
|
/rtm |
3 participants
Description
Installationto be language-aware instead of language-lockedAvailableOperationsbased on the presence of AsyncAPI operations,not on a specific language
Related issue(s)
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.