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 language prop and selects an install command from a map; Readme always renders Installation (passing language) and fetches operations unconditionally, rendering AvailableOperations only if operations exist; MessageExamples generates per-language examples via language-specific renderers for JavaScript and Python. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)packages/components/src/components/readme/MessageExamples.js (1)
⏰ 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)
🔇 Additional comments (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!
| python: { | ||
| label: 'Python', | ||
| codeBlock: 'python', | ||
| render: (operationId, payload) => ` | ||
| **Example (Python):** | ||
| \`\`\`python | ||
| client.${operationId}(${JSON.stringify(payload, null, 2)}) | ||
| \`\`\` | ||
| ` |
There was a problem hiding this comment.
For python you can look at generated clients and the methods name is in snakecase but in your implementation it is camelCase which is used in js. So you can use from @asyncapi/generator-helpers https://github.com/asyncapi/generator/blob/master/packages/helpers/src/utils.js#L70 to convert operationId.
| const payload = example.payload(); | ||
| messageExamples.push(exampleTemplate(operationId, payload)); | ||
| Object.values(languageConfig).forEach(({ render }) => { | ||
| messageExamples.push(render(operationId, payload)); |
There was a problem hiding this comment.
you are only using render function from the lang config than why you label and codeBlock properties for every lang in config?
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.