refactor: update getQueryParams to support multiple channels#2026
Open
SushanthMusham wants to merge 3 commits intoasyncapi:masterfrom
Open
refactor: update getQueryParams to support multiple channels#2026SushanthMusham wants to merge 3 commits intoasyncapi:masterfrom
SushanthMusham wants to merge 3 commits intoasyncapi:masterfrom
Conversation
…template compatibility
SushanthMusham
requested review from
Adi-204,
asyncapi-bot-eve,
derberg,
jonaslagoni and
magicmatatjahu
as code owners
|
Contributor
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.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAggregates WebSocket query parameter defaults across all channels, changes getQueryParams to return a per-channel object, adds getFirstChannelQueryParams (first channel → Map), and updates templates, components, and tests to consume the new shapes and helper. No public signatures were removed. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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🧪 Generate unit tests (beta)
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: 5
🧹 Nitpick comments (7)
packages/templates/clients/websocket/python/test/components/QueryParamsArgumentsDocs.test.js (1)
23-26: Guard empty-channel results before first-channel extraction.If
getQueryParamsreturns an empty object, Line 24 producesundefinedand Line 25 will throw onObject.entries(undefined). Add a key-length check before indexing.Suggested fix
let queryParamsArray = null; -if (allQueryParams) { - const firstChannelName = Object.keys(allQueryParams)[0]; - queryParamsArray = Object.entries(allQueryParams[firstChannelName]); +if (allQueryParams && Object.keys(allQueryParams).length > 0) { + const firstChannelName = Object.keys(allQueryParams)[0]; + const firstChannelQueryParams = allQueryParams[firstChannelName]; + queryParamsArray = firstChannelQueryParams ? Object.entries(firstChannelQueryParams) : null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/templates/clients/websocket/python/test/components/QueryParamsArgumentsDocs.test.js` around lines 23 - 26, The code assumes allQueryParams has at least one key and does Object.keys(allQueryParams)[0] then Object.entries(...) which will throw if allQueryParams is empty; update the block that sets firstChannelName and queryParamsArray to first check that Object.keys(allQueryParams).length > 0 (or that firstChannelName is truthy) before indexing and calling Object.entries, and only set queryParamsArray when a channel exists (leave it empty or default otherwise) so references to firstChannelName and queryParamsArray are safe; target the variables allQueryParams, firstChannelName and queryParamsArray in the test.packages/templates/clients/websocket/python/test/components/Requires.test.js (1)
20-27: Consider adding a defensive check for emptyallQueryParams.If
getQueryParamsreturns an empty object{},Object.keys(allQueryParams)[0]will beundefined, causingObject.entries(allQueryParams[undefined])to throw aTypeError. While the fixture presumably contains query params, a small guard would make this more robust:🛡️ Proposed defensive fix
const allQueryParams = getQueryParams(channels); // Convert the first channel's parameters back into a Map so the test doesn't break let queryParams = null; -if (allQueryParams) { +if (allQueryParams && Object.keys(allQueryParams).length > 0) { const firstChannelName = Object.keys(allQueryParams)[0]; queryParams = new Map(Object.entries(allQueryParams[firstChannelName])); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/templates/clients/websocket/python/test/components/Requires.test.js` around lines 20 - 27, The test assumes getQueryParams(channels) returns a non-empty object and directly accesses Object.keys(allQueryParams)[0], which can be undefined; update the block around allQueryParams in Requires.test.js to defensively handle an empty result by checking that allQueryParams is truthy and has at least one key before accessing the first channel (e.g., use const keys = Object.keys(allQueryParams); if (allQueryParams && keys.length > 0) { const firstChannelName = keys[0]; queryParams = new Map(Object.entries(allQueryParams[firstChannelName] || {})); } ), ensuring you reference getQueryParams, allQueryParams, and queryParams in the change.packages/templates/clients/websocket/python/components/ClientClass.js (1)
10-71: Consider adding JSDoc for the exported function.The
ClientClassfunction is exported but lacks JSDoc documentation. As per coding guidelines, functions should include clear JSDoc comments with parameter types and return values.📝 Suggested JSDoc addition
+/** + * Renders the Python WebSocket client class. + * + * `@param` {Object} props - Component props. + * `@param` {Object} props.asyncapi - The parsed AsyncAPI document. + * `@param` {Object} props.params - Template parameters including server, appendClientSuffix, and customClientName. + * `@returns` {JSX.Element} The rendered Python client class. + */ export function ClientClass({ asyncapi, params }) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/templates/clients/websocket/python/components/ClientClass.js` around lines 10 - 71, Add a JSDoc block above the exported function ClientClass that documents the function, its parameters and return type: describe the first param asyncapi as the AsyncAPI document/object (type AsyncAPI or any), the second param params as an options object documenting expected fields (e.g. server, appendClientSuffix, customClientName), and state the function returns a JSX/React/Text element (or string template fragment) used to render the client class; reference the function name ClientClass and the parameter names asyncapi and params so the comment is placed directly above export function ClientClass({ asyncapi, params }) and follows project JSDoc style.packages/templates/clients/websocket/java/quarkus/template/src/main/java/com/asyncapi/connector.java.js (2)
6-30: Consider adding JSDoc for the exported function.The default exported async function lacks JSDoc documentation describing its parameters and return value.
📝 Suggested JSDoc addition
+/** + * Generates the Java Quarkus WebSocket connector file. + * + * `@param` {Object} props - Generator props. + * `@param` {Object} props.asyncapi - The parsed AsyncAPI document. + * `@param` {Object} props.params - Template parameters including server, appendClientSuffix, and customClientName. + * `@returns` {Promise<JSX.Element>} The rendered connector file. + */ export default async function ({ asyncapi, params }) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/templates/clients/websocket/java/quarkus/template/src/main/java/com/asyncapi/connector.java.js` around lines 6 - 30, Add a JSDoc block above the default exported async function to document the input parameters and return value: describe the function as async and list the shape/meaning of the single destructured parameter ({ asyncapi, params }) including expected params fields (server, appendClientSuffix, customClientName), note that it uses helpers getServer/getClientName/getQueryParams and returns a JSX File element containing ConnectorDependencies and ClientConnector (i.e., a template fragment), and document any thrown errors or side effects if applicable.
9-18: Consider extracting the repeated bridge logic into a shared helper.The same "first channel to Map" conversion pattern is duplicated across multiple template files (Python
ClientClass.js, Javaconnector.java.js,client.java.js, etc.). Consider adding a utility function in@asyncapi/generator-helpersto reduce duplication:// Example helper in generator-helpers function getFirstChannelQueryParamsAsMap(channels) { const allQueryParams = getQueryParams(channels); if (!allQueryParams) return null; const firstChannelName = Object.keys(allQueryParams)[0]; return new Map(Object.entries(allQueryParams[firstChannelName])); }This would simplify all consuming code to a single function call while preserving backward compatibility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/templates/clients/websocket/java/quarkus/template/src/main/java/com/asyncapi/connector.java.js` around lines 9 - 18, Extract the duplicated "first channel to Map" logic into a shared helper called getFirstChannelQueryParamsAsMap in `@asyncapi/generator-helpers`: implement a function that calls getQueryParams(channels), returns null if falsy, otherwise picks the first channel key and returns new Map(Object.entries(...)); then replace the inline pattern in connector.java.js (the allQueryParams variable and subsequent Map construction that sets queryParams) with a single call to getFirstChannelQueryParamsAsMap(asyncapi.channels()) to keep behavior identical and remove duplication.packages/components/test/components/QueryParamsVariables.test.js (1)
22-26: Consider extracting the repeated first-channel conversion into a test helper.The same block appears 3 times; centralizing it will reduce drift and keep future shape changes easier to apply.
♻️ Refactor sketch
+function getFirstChannelQueryParamsArray(channels) { + const queryParamsObject = getQueryParams(channels); + if (!queryParamsObject || Object.keys(queryParamsObject).length === 0) return []; + const firstChannelName = Object.keys(queryParamsObject)[0]; + return Object.entries(queryParamsObject[firstChannelName] ?? {}); +} ... - const queryParamsObject = getQueryParams(channels); - let queryParamsArray = []; - if (queryParamsObject) { - const firstChannelName = Object.keys(queryParamsObject)[0]; - queryParamsArray = Object.entries(queryParamsObject[firstChannelName]); - } + const queryParamsArray = getFirstChannelQueryParamsArray(channels);Also applies to: 41-45, 60-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/components/test/components/QueryParamsVariables.test.js` around lines 22 - 26, Extract the repeated conversion logic for test fixtures into a small test helper (e.g., getQueryParamsArray) and replace the three identical blocks that compute queryParamsArray from queryParamsObject using firstChannelName/ Object.entries with a single call to that helper; the helper should accept queryParamsObject, guard for falsy input, compute Object.keys(queryParamsObject)[0] as firstChannelName, and return Object.entries(queryParamsObject[firstChannelName]) so tests use a single source of truth for this transformation.packages/templates/clients/websocket/java/quarkus/test/components/InitConnector.test.js (1)
21-30: LGTM with optional defensive improvement.The two-step extraction pattern correctly adapts to the new multi-channel
getQueryParamsAPI. For test code with a known fixture, this is acceptable.Optional: Add a guard for the empty object edge case if you want the pattern to be more robust for future reuse:
🛡️ Optional defensive guard
// Extract the parameters from the first channel into an array for the tests queryParamsArray = null; - if (allQueryParams) { + if (allQueryParams && Object.keys(allQueryParams).length > 0) { const firstChannelName = Object.keys(allQueryParams)[0]; queryParamsArray = Object.entries(allQueryParams[firstChannelName]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/templates/clients/websocket/java/quarkus/test/components/InitConnector.test.js` around lines 21 - 30, The test currently assumes allQueryParams has at least one channel before reading Object.keys(allQueryParams)[0]; add a defensive guard around the extraction of firstChannelName and building queryParamsArray: check that allQueryParams is an object and has at least one key (non-empty) before calling Object.keys(...) and Object.entries(...), otherwise set queryParamsArray to an empty array or a sensible default; update the code around getQueryParams, allQueryParams, firstChannelName, and queryParamsArray to handle the empty-object case safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/components/src/components/QueryParamsVariables.js`:
- Around line 116-145: The JSDoc example for the QueryParamsVariables component
contains malformed comment markers (many instances of "* *") which breaks
rendering; edit the JSDoc example block for QueryParamsVariables in
QueryParamsVariables.js and replace all occurrences of "* *" at the start of
example lines with a single "*" so each example line is a proper JSDoc line
(preserve spacing/indentation and all example content, only fix the duplicated
asterisks).
In `@packages/helpers/src/bindings.js`:
- Around line 14-59: Update the JSDoc for getQueryParams to specify that the
channels parameter is an AsyncAPI Channels collection (not a plain Object) that
implements .isEmpty() and .all(), and document assumptions and error conditions:
note that the function expects channels to be non-null and to provide .isEmpty()
and .all() methods (or it will return null/skip processing), and describe return
values (object mapping channelName -> params or null when no ws query params
found). Reference getQueryParams(channels), the channels .isEmpty() and .all()
usage, and the expectation that channel bindings provide a ws binding with
.value()?.query.
In
`@packages/templates/clients/websocket/java/quarkus/template/src/main/java/com/asyncapi/client.java.js`:
- Around line 17-20: The current block uses Object.keys(allQueryParams)[0]
without ensuring there is at least one key, which can throw; update the guard
around allQueryParams to also check that Object.keys(allQueryParams).length > 0
and that the resolved firstChannelName entry exists before converting to a Map.
Specifically, change the conditional that references
allQueryParams/firstChannelName and ensure queryParams = new
Map(Object.entries(allQueryParams[firstChannelName])) only runs when a non-empty
key list exists and allQueryParams[firstChannelName] is truthy.
In `@packages/templates/clients/websocket/python/template/client.py.js`:
- Around line 16-19: The template assumes allQueryParams has at least one key
and does Map(Object.entries(...)) unguarded; add a guard to ensure a
firstChannelName and a defined allQueryParams[firstChannelName] before
converting to a Map and otherwise set queryParams to an empty Map. Specifically,
update the block that references allQueryParams, firstChannelName and
queryParams so it checks Object.keys(allQueryParams).length > 0 (or that
firstChannelName is truthy and allQueryParams[firstChannelName] !== undefined)
before calling Object.entries, and fallback to new Map() when there are no
entries.
In
`@packages/templates/clients/websocket/python/test/components/InitSignature.test.js`:
- Around line 60-63: The code assumes allQueryParams has at least one key before
calling Object.entries and can throw when allQueryParams is an empty object;
change the guard to ensure there is at least one channel and that the first
channel's params exist before calling Object.entries (e.g., check
Object.keys(allQueryParams).length > 0 and that allQueryParams[firstChannelName]
is defined) so queryParamsArray is only assigned from Object.entries when a
valid object exists (referencing allQueryParams, firstChannelName, and
queryParamsArray in InitSignature.test.js).
---
Nitpick comments:
In `@packages/components/test/components/QueryParamsVariables.test.js`:
- Around line 22-26: Extract the repeated conversion logic for test fixtures
into a small test helper (e.g., getQueryParamsArray) and replace the three
identical blocks that compute queryParamsArray from queryParamsObject using
firstChannelName/ Object.entries with a single call to that helper; the helper
should accept queryParamsObject, guard for falsy input, compute
Object.keys(queryParamsObject)[0] as firstChannelName, and return
Object.entries(queryParamsObject[firstChannelName]) so tests use a single source
of truth for this transformation.
In
`@packages/templates/clients/websocket/java/quarkus/template/src/main/java/com/asyncapi/connector.java.js`:
- Around line 6-30: Add a JSDoc block above the default exported async function
to document the input parameters and return value: describe the function as
async and list the shape/meaning of the single destructured parameter ({
asyncapi, params }) including expected params fields (server,
appendClientSuffix, customClientName), note that it uses helpers
getServer/getClientName/getQueryParams and returns a JSX File element containing
ConnectorDependencies and ClientConnector (i.e., a template fragment), and
document any thrown errors or side effects if applicable.
- Around line 9-18: Extract the duplicated "first channel to Map" logic into a
shared helper called getFirstChannelQueryParamsAsMap in
`@asyncapi/generator-helpers`: implement a function that calls
getQueryParams(channels), returns null if falsy, otherwise picks the first
channel key and returns new Map(Object.entries(...)); then replace the inline
pattern in connector.java.js (the allQueryParams variable and subsequent Map
construction that sets queryParams) with a single call to
getFirstChannelQueryParamsAsMap(asyncapi.channels()) to keep behavior identical
and remove duplication.
In
`@packages/templates/clients/websocket/java/quarkus/test/components/InitConnector.test.js`:
- Around line 21-30: The test currently assumes allQueryParams has at least one
channel before reading Object.keys(allQueryParams)[0]; add a defensive guard
around the extraction of firstChannelName and building queryParamsArray: check
that allQueryParams is an object and has at least one key (non-empty) before
calling Object.keys(...) and Object.entries(...), otherwise set queryParamsArray
to an empty array or a sensible default; update the code around getQueryParams,
allQueryParams, firstChannelName, and queryParamsArray to handle the
empty-object case safely.
In `@packages/templates/clients/websocket/python/components/ClientClass.js`:
- Around line 10-71: Add a JSDoc block above the exported function ClientClass
that documents the function, its parameters and return type: describe the first
param asyncapi as the AsyncAPI document/object (type AsyncAPI or any), the
second param params as an options object documenting expected fields (e.g.
server, appendClientSuffix, customClientName), and state the function returns a
JSX/React/Text element (or string template fragment) used to render the client
class; reference the function name ClientClass and the parameter names asyncapi
and params so the comment is placed directly above export function ClientClass({
asyncapi, params }) and follows project JSDoc style.
In
`@packages/templates/clients/websocket/python/test/components/QueryParamsArgumentsDocs.test.js`:
- Around line 23-26: The code assumes allQueryParams has at least one key and
does Object.keys(allQueryParams)[0] then Object.entries(...) which will throw if
allQueryParams is empty; update the block that sets firstChannelName and
queryParamsArray to first check that Object.keys(allQueryParams).length > 0 (or
that firstChannelName is truthy) before indexing and calling Object.entries, and
only set queryParamsArray when a channel exists (leave it empty or default
otherwise) so references to firstChannelName and queryParamsArray are safe;
target the variables allQueryParams, firstChannelName and queryParamsArray in
the test.
In
`@packages/templates/clients/websocket/python/test/components/Requires.test.js`:
- Around line 20-27: The test assumes getQueryParams(channels) returns a
non-empty object and directly accesses Object.keys(allQueryParams)[0], which can
be undefined; update the block around allQueryParams in Requires.test.js to
defensively handle an empty result by checking that allQueryParams is truthy and
has at least one key before accessing the first channel (e.g., use const keys =
Object.keys(allQueryParams); if (allQueryParams && keys.length > 0) { const
firstChannelName = keys[0]; queryParams = new
Map(Object.entries(allQueryParams[firstChannelName] || {})); } ), ensuring you
reference getQueryParams, allQueryParams, and queryParams in the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2148922a-f2a0-4cea-ace3-8a4caf5969d3
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (22)
packages/components/src/components/QueryParamsVariables.jspackages/components/test/components/QueryParamsVariables.test.jspackages/helpers/src/bindings.jspackages/helpers/test/bindings.test.jspackages/templates/clients/websocket/java/quarkus/template/src/main/java/com/asyncapi/client.java.jspackages/templates/clients/websocket/java/quarkus/template/src/main/java/com/asyncapi/connector.java.jspackages/templates/clients/websocket/java/quarkus/test/components/ClientConnector.test.jspackages/templates/clients/websocket/java/quarkus/test/components/ClientDependencies.test.jspackages/templates/clients/websocket/java/quarkus/test/components/ClientFields.test.jspackages/templates/clients/websocket/java/quarkus/test/components/ConnectorDependencies.test.jspackages/templates/clients/websocket/java/quarkus/test/components/Constructor.test.jspackages/templates/clients/websocket/java/quarkus/test/components/ConstructorSignature.test.jspackages/templates/clients/websocket/java/quarkus/test/components/DefaultConstructorSignature.test.jspackages/templates/clients/websocket/java/quarkus/test/components/EchoWebSocket.test.jspackages/templates/clients/websocket/java/quarkus/test/components/InitConnector.test.jspackages/templates/clients/websocket/java/quarkus/test/components/URIParams.test.jspackages/templates/clients/websocket/python/components/ClientClass.jspackages/templates/clients/websocket/python/template/client.py.jspackages/templates/clients/websocket/python/test/components/Constructor.test.jspackages/templates/clients/websocket/python/test/components/InitSignature.test.jspackages/templates/clients/websocket/python/test/components/QueryParamsArgumentsDocs.test.jspackages/templates/clients/websocket/python/test/components/Requires.test.js
.../templates/clients/websocket/java/quarkus/template/src/main/java/com/asyncapi/client.java.js
Outdated
Show resolved
Hide resolved
Comment on lines
+16
to
+19
| if (allQueryParams) { | ||
| const firstChannelName = Object.keys(allQueryParams)[0]; | ||
| queryParams = new Map(Object.entries(allQueryParams[firstChannelName])); | ||
| } |
There was a problem hiding this comment.
Add an empty-result guard before Map conversion.
If allQueryParams is {}, this currently calls Object.entries(undefined) and throws during template generation.
🛠️ Proposed hardening
let queryParams = null;
- if (allQueryParams) {
- const firstChannelName = Object.keys(allQueryParams)[0];
- queryParams = new Map(Object.entries(allQueryParams[firstChannelName]));
+ if (allQueryParams && Object.keys(allQueryParams).length > 0) {
+ const firstChannelName = Object.keys(allQueryParams)[0];
+ const firstChannelParams = allQueryParams[firstChannelName];
+ if (firstChannelParams) {
+ queryParams = new Map(Object.entries(firstChannelParams));
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/templates/clients/websocket/python/template/client.py.js` around
lines 16 - 19, The template assumes allQueryParams has at least one key and does
Map(Object.entries(...)) unguarded; add a guard to ensure a firstChannelName and
a defined allQueryParams[firstChannelName] before converting to a Map and
otherwise set queryParams to an empty Map. Specifically, update the block that
references allQueryParams, firstChannelName and queryParams so it checks
Object.keys(allQueryParams).length > 0 (or that firstChannelName is truthy and
allQueryParams[firstChannelName] !== undefined) before calling Object.entries,
and fallback to new Map() when there are no entries.
Member
There was a problem hiding this comment.
@SushanthMusham look at this suggestion and apply it
Comment on lines
+60
to
+63
| if (allQueryParams) { | ||
| const firstChannelName = Object.keys(allQueryParams)[0]; | ||
| queryParamsArray = Object.entries(allQueryParams[firstChannelName]); | ||
| } |
There was a problem hiding this comment.
Guard against empty allQueryParams before calling Object.entries.
Line 60 only checks truthiness. If allQueryParams is {}, Line 62 evaluates Object.entries(undefined) and throws.
Suggested fix
- let queryParamsArray = null;
- if (allQueryParams) {
- const firstChannelName = Object.keys(allQueryParams)[0];
- queryParamsArray = Object.entries(allQueryParams[firstChannelName]);
- }
+ let queryParamsArray = null;
+ if (allQueryParams) {
+ const channelNames = Object.keys(allQueryParams);
+ if (channelNames.length > 0) {
+ const firstChannelName = channelNames[0];
+ queryParamsArray = Object.entries(allQueryParams[firstChannelName] ?? {});
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/templates/clients/websocket/python/test/components/InitSignature.test.js`
around lines 60 - 63, The code assumes allQueryParams has at least one key
before calling Object.entries and can throw when allQueryParams is an empty
object; change the guard to ensure there is at least one channel and that the
first channel's params exist before calling Object.entries (e.g., check
Object.keys(allQueryParams).length > 0 and that allQueryParams[firstChannelName]
is defined) so queryParamsArray is only assigned from Object.entries when a
valid object exists (referencing allQueryParams, firstChannelName, and
queryParamsArray in InitSignature.test.js).
Member
There was a problem hiding this comment.
@SushanthMusham look at this suggestion and apply it
Adi-204
reviewed
Mar 24, 2026
Member
Adi-204
left a comment
Adi-204
left a comment
There was a problem hiding this comment.
@SushanthMusham, the only concern I have with the current refactoring is the repeated pattern below:
// Fetch all channel parameters with our new helper
const allQueryParams = getQueryParams(channels);
// Convert the first channel's parameters back into a Map for the tests
queryParams = null;
if (allQueryParams) {
const firstChannelName = Object.keys(allQueryParams)[0];
queryParams = new Map(Object.entries(allQueryParams[firstChannelName]));
}This logic is now duplicated across multiple components due to the helper change, which makes the code a bit redundant and harder to maintain.
I think it would be better to introduce an additional helper that directly returns the parameters for the first channel (in the required format). That way, we can avoid repetition and keep the components cleaner.
Also address the coderabbit comments it is much better now and give nice suggestions.
Author
|
I’ve completed the refactor as suggested.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/helpers/src/bindings.js`:
- Around line 19-55: The code uses plain object literals for document-controlled
keys which allows prototype pollution; replace the mutable plain objects with
null-prototype objects by creating result and the per-channel channelParams
using Object.create(null) instead of {} (references: result, channelParams,
channelName, properties, channels.all(), and the loop that assigns query
property names) so that assigning document-derived keys cannot mutate
Object.prototype.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 898b5c3d-e907-4d4d-82b2-42d1c74191b4
📒 Files selected for processing (17)
packages/components/src/components/QueryParamsVariables.jspackages/helpers/src/bindings.jspackages/helpers/src/index.jspackages/templates/clients/websocket/java/quarkus/template/src/main/java/com/asyncapi/client.java.jspackages/templates/clients/websocket/java/quarkus/template/src/main/java/com/asyncapi/connector.java.jspackages/templates/clients/websocket/java/quarkus/test/components/ClientConnector.test.jspackages/templates/clients/websocket/java/quarkus/test/components/ClientDependencies.test.jspackages/templates/clients/websocket/java/quarkus/test/components/ClientFields.test.jspackages/templates/clients/websocket/java/quarkus/test/components/ConnectorDependencies.test.jspackages/templates/clients/websocket/java/quarkus/test/components/Constructor.test.jspackages/templates/clients/websocket/java/quarkus/test/components/ConstructorSignature.test.jspackages/templates/clients/websocket/java/quarkus/test/components/DefaultConstructorSignature.test.jspackages/templates/clients/websocket/java/quarkus/test/components/EchoWebSocket.test.jspackages/templates/clients/websocket/java/quarkus/test/components/InitConnector.test.jspackages/templates/clients/websocket/java/quarkus/test/components/URIParams.test.jspackages/templates/clients/websocket/python/template/client.py.jspackages/templates/clients/websocket/python/test/components/InitSignature.test.js
✅ Files skipped from review due to trivial changes (4)
- packages/templates/clients/websocket/java/quarkus/test/components/Constructor.test.js
- packages/templates/clients/websocket/java/quarkus/test/components/URIParams.test.js
- packages/templates/clients/websocket/java/quarkus/test/components/ConnectorDependencies.test.js
- packages/components/src/components/QueryParamsVariables.js
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/templates/clients/websocket/java/quarkus/template/src/main/java/com/asyncapi/client.java.js
- packages/templates/clients/websocket/java/quarkus/test/components/EchoWebSocket.test.js
- packages/templates/clients/websocket/java/quarkus/test/components/ConstructorSignature.test.js
- packages/templates/clients/websocket/java/quarkus/test/components/DefaultConstructorSignature.test.js
- packages/templates/clients/websocket/java/quarkus/test/components/InitConnector.test.js
- packages/templates/clients/websocket/python/test/components/InitSignature.test.js
- packages/templates/clients/websocket/java/quarkus/test/components/ClientDependencies.test.js
- packages/templates/clients/websocket/python/template/client.py.js
- packages/templates/clients/websocket/java/quarkus/template/src/main/java/com/asyncapi/connector.java.js
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR refactors the
getQueryParamshelper function to return query parameters for all available channels, rather than just the first one. This is a foundational change to support multi-channel WebSocket/Kafka generations.Changes:
getQueryParamsin@asyncapi/generator-helpersto return an object keyed by channel names.@asyncapi/generator-react-sdk(QueryParamsVariables component)core-template-client-websocket-pythoncore-template-client-websocket-java-quarkusRelated Issue(s)
Fixes #1973
Checklist
npm test.Summary by CodeRabbit
Refactor
Templates
Tests
Documentation