feat: move HandleMessage component to @asyncapi/generator-components
#1713
Conversation
Adi-204
requested review from
derberg,
magicmatatjahu and
asyncapi-bot-eve
as code owners
|
|
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 WalkthroughAdds a new language-aware HandleMessage component in generator-components, re-exports it publicly, adds snapshot tests, removes per-template HandleMessage implementations, and updates Dart/JavaScript/Python WebSocket client templates to import and use the shared component with explicit props. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests
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 |
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.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
packages/components/src/index.js (1)
6-7: Document the new public surface.
Please add HandleMessage (and MethodGenerator if new) to the package README/CHANGELOG to avoid silent API drift.packages/templates/clients/websocket/javascript/components/ClientClass.js (1)
22-25: Verify cb is wired at call sites; align semantics with RegisterMessageHandler.
HandleMessage expects (message, cb) while RegisterMessageHandler registers a handler. Ensure the websocket onmessage path passes a dispatcher cb that invokes all registered handlers; otherwise messages will be dropped.If no cb is provided, consider a safe fallback (log raw message) to mirror Python.
packages/components/test/components/HandleMessage.test.js (2)
13-23: Minor: keep docstring tidy.
Drop the trailing space inside the triple-quoted string to reduce snapshot churn.- preExecutionCode='"""Pass the incoming message to all registered message handlers. """' + preExecutionCode='"""Pass the incoming message to all registered message handlers."""'
25-35: Add negative test for unsupported language (after adding guard).
Covers error path and improves robustness.test('render dart websocket message handler method', () => { @@ }); + + test('throws on unsupported language', () => { + expect(() => render(<HandleMessage language="go" />)).toThrow(/Unsupported language/i); + });packages/templates/clients/websocket/dart/components/ClientClass.js (1)
23-27: Confirm dispatcher cb is provided on message receive; consider null-safe fallback.
_handleMessage expects a non-null cb. Verify Connect/onMessage provides it; otherwise add a guard to log the raw message.packages/components/src/components/HandleMessage.js (2)
47-58: Expose formatting knobs via props for consistency with MethodGenerator.
Let callers override indent/newLines/customMethodConfig when needed.<MethodGenerator language={language} methodName={methodName} methodParams={methodParams} methodDocs={methodDocs} methodLogic={methodLogic} preExecutionCode={preExecutionCode} postExecutionCode={postExecutionCode} - indent={2} - newLines={2} + indent={indent} + newLines={newLines} + customMethodConfig={customMethodConfig} />
13-19: Nit: normalize Python indentation for readability.
Two-space indent under if is unconventional; align to 4 spaces for consistency with the else/for block.python: { - methodLogic: `if len(self.message_handlers) == 0: - print("\\033[94mReceived raw message:\\033[0m", message) -else: - for handler in self.message_handlers: - handler(message)` + methodLogic: `if len(self.message_handlers) == 0: + print("\\033[94mReceived raw message:\\033[0m", message) +else: + for handler in self.message_handlers: + handler(message)` },packages/templates/clients/websocket/python/components/ClientClass.js (1)
35-40: Trim trailing space in Python docstring.Minor PEP 257 polish.
Apply:
- preExecutionCode='"""Pass the incoming message to all registered message handlers. """' + preExecutionCode='"""Pass the incoming message to all registered message handlers."""'Confirm
MethodGeneratoremitspreExecutionCodeas the first statement in the method body so it becomes the function’s__doc__(and thatmethodDocsisn’t inserted before it).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
packages/components/test/components/__snapshots__/HandleMessage.test.js.snapis excluded by!**/*.snappackages/templates/clients/websocket/test/integration-test/__snapshots__/integration.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (9)
packages/components/src/components/HandleMessage.js(1 hunks)packages/components/src/index.js(1 hunks)packages/components/test/components/HandleMessage.test.js(1 hunks)packages/templates/clients/websocket/dart/components/ClientClass.js(2 hunks)packages/templates/clients/websocket/dart/components/HandleMessage.js(0 hunks)packages/templates/clients/websocket/javascript/components/ClientClass.js(2 hunks)packages/templates/clients/websocket/javascript/components/HandleMessage.js(0 hunks)packages/templates/clients/websocket/python/components/ClientClass.js(2 hunks)packages/templates/clients/websocket/python/components/HandleMessage.js(0 hunks)
💤 Files with no reviewable changes (3)
- packages/templates/clients/websocket/javascript/components/HandleMessage.js
- packages/templates/clients/websocket/dart/components/HandleMessage.js
- packages/templates/clients/websocket/python/components/HandleMessage.js
🧰 Additional context used
🧬 Code graph analysis (5)
packages/components/test/components/HandleMessage.test.js (2)
apps/react-sdk/src/renderer/renderer.ts (1)
render(63-77)packages/components/src/components/HandleMessage.js (1)
HandleMessage(41-60)
packages/components/src/components/HandleMessage.js (1)
packages/components/src/components/MethodGenerator.js (1)
MethodGenerator(34-82)
packages/templates/clients/websocket/dart/components/ClientClass.js (1)
packages/components/src/components/HandleMessage.js (1)
HandleMessage(41-60)
packages/templates/clients/websocket/javascript/components/ClientClass.js (1)
packages/components/src/components/HandleMessage.js (1)
HandleMessage(41-60)
packages/templates/clients/websocket/python/components/ClientClass.js (1)
packages/components/src/components/HandleMessage.js (1)
HandleMessage(41-60)
⏰ 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). (6)
- GitHub Check: Test generator as dependency with Node 20
- GitHub Check: Test generator as dependency with Node 18
- GitHub Check: Acceptance tests for generated templates
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - macos-13
- GitHub Check: Test NodeJS PR - ubuntu-latest
🔇 Additional comments (5)
packages/components/src/index.js (1)
6-7: LGTM: public re-exports added correctly.packages/templates/clients/websocket/javascript/components/ClientClass.js (1)
6-6: LGTM: switched to central components import.packages/components/test/components/HandleMessage.test.js (2)
1-3: LGTM: tests target the public export and renderer.
5-35: Ensure snapshots are committed or switch to inline snapshots.
These tests use toMatchSnapshot; please commit the generated snapshots or convert to inline snapshots to avoid CI failures.packages/templates/clients/websocket/dart/components/ClientClass.js (1)
6-6: LGTM: centralized imports.
| import { SendOperation } from './SendOperation'; | ||
| import { Send } from './Send'; | ||
| import { CloseConnection, RegisterMessageHandler } from '@asyncapi/generator-components'; | ||
| import { CloseConnection, RegisterMessageHandler, HandleMessage } from '@asyncapi/generator-components'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Switch to shared HandleMessage import looks good—verify export and dependency.
Ensure @asyncapi/generator-components re-exports HandleMessage and that the repo depends on a version including it. Also confirm old template-local HandleMessage.js files are gone to avoid dead code.
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Check HandleMessage export in components index:"
rg -nP 'export\s+\{[^}]*HandleMessage' packages/components/src/index.js || rg -n 'export \* from .*/HandleMessage' packages/components/src/index.js || true
echo -e "\n2) Ensure no leftover template-local HandleMessage.js files:"
fd -a -t f --exclude node_modules 'HandleMessage.js' packages/templates || true
echo -e "\n3) Check dependency declarations for generator-components:"
rg -n '"@asyncapi/generator-components"' package.json packages/**/package.json || true
echo -e "\n4) Verify imports now point to @asyncapi/generator-components:"
rg -nP "from\\s+'@asyncapi/generator-components'.*HandleMessage" packages/templates || trueLength of output: 1045
Add missing @asyncapi/generator-components dependency
The export of HandleMessage is confirmed and no stale template-local files remain, but the templates that import it don’t declare @asyncapi/generator-components. Add it (with a version including HandleMessage) to the relevant package.json (e.g. packages/templates/clients/websocket/python/package.json) so the import resolves.
🤖 Prompt for AI Agents
In packages/templates/clients/websocket/python/components/ClientClass.js around
line 8, the file imports HandleMessage (and other symbols) from
'@asyncapi/generator-components' but the template package does not declare this
dependency; update packages/templates/clients/websocket/python/package.json to
add '@asyncapi/generator-components' with a version range that includes the
release containing HandleMessage (e.g. bump to the minimum semver that
introduced HandleMessage or use a caret range like ^X.Y.Z), then run npm/yarn
install and verify the import resolves and the build/tests for the template
pass.
|
Description
Add new component
HandleMessageand refactor existing codebase.Related issue(s)
Fixes #1711
Summary by CodeRabbit
New Features
Refactor
Tests