feat: add query Parameters support to javascript websocket client - #2142
feat: add query Parameters support to javascript websocket client#2142batchu5 wants to merge 5 commits into
Conversation
|
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 ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds query parameter support to the JavaScript WebSocket client template. The template extracts parameters from channel bindings, generates constructor signatures and documentation, appends query strings to WebSocket URLs, and wires the data through client generation and tests. ChangesQuery parameter support in JS WebSocket client
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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: 4
🧹 Nitpick comments (2)
packages/templates/clients/websocket/javascript/test/components/InitSignature.test.js (2)
28-38: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for a query param without a default value.
Current tests only cover truthy default values (
'false','true') and the multi-param fixture case. The component'sdefaultValue = paramDefaultValue ? ... : ''branch (param present but no default) isn't exercised by any test.✅ Suggested additional test
test('renders with single query parameter with default value true', () => { const queryParamsWithTrueDefault = [['bids', 'true']]; const result = render(<InitSignature queryParams={queryParamsWithTrueDefault} />); expect(result.trim()).toMatchSnapshot(); }); + + test('renders with single query parameter without default value', () => { + const queryParamsWithoutDefault = [['token', undefined]]; + const result = render(<InitSignature queryParams={queryParamsWithoutDefault} />); + expect(result.trim()).toMatchSnapshot(); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/templates/clients/websocket/javascript/test/components/InitSignature.test.js` around lines 28 - 38, Add a test case in InitSignature.test.js to cover a query param entry without a default value, since InitSignature’s defaultValue handling currently only has coverage through the truthy default branches and the multi-param fixture. Extend the existing render snapshot tests for InitSignature so one case passes a param tuple with no second element and asserts the rendered output, exercising the paramDefaultValue ? ... : '' path.
1-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated parser/fixture setup across test files.
The parser instantiation, fixture path, and
beforeAllblock (lines 1-16) are identical toQueryParamsArgumentsDocs.test.js. Consider extracting a shared test helper to load the parsed document once for both test files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/templates/clients/websocket/javascript/test/components/InitSignature.test.js` around lines 1 - 16, The parser setup in InitSignature.test.js is duplicated from QueryParamsArgumentsDocs.test.js, so extract the shared AsyncAPI document loading logic into a common test helper. Move the repeated Parser/fromFile fixture path and beforeAll parsing flow into a reusable utility, then have InitSignature and QueryParamsArgumentsDocs import and use it so the parsed document is created in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/templates/clients/websocket/javascript/components/InitSignature.js`:
- Around line 12-17: The query param signature generation in InitSignature.js is
treating every default as a quoted string and dropping falsy defaults entirely.
Update the default rendering logic in queryParams.map so it checks whether
paramDefaultValue is actually provided (for example, not undefined/null) instead
of using a truthy test, and preserve boolean/number defaults without forcing
string quotes. Also align the guard behavior in QueryParamsVariables.js so
appending to the URL does not depend on JS truthiness of a stringified default.
- Around line 1-24: Add a clear JSDoc block for the exported InitSignature
function to match the repo guidelines. Document the queryParams argument with
its expected shape, describe the returned Text output, and note any relevant
edge cases or error conditions. Place the comment directly above InitSignature
so it stays with the function even if the component is moved or refactored.
- Around line 3-24: Sanitize the query-param names before they are interpolated
into the constructor signature in InitSignature; right now paramName is emitted
directly, which can produce invalid JS identifiers or collide with reserved
parameters like url and throwSendErrors. Update InitSignature (and the matching
QueryParamsVariables component) to map each original query key to a safe
parameter name first, while preserving the original key for lookups and
defaults, so generated constructor syntax stays valid and unique.
In
`@packages/templates/clients/websocket/javascript/components/QueryParamsArgumentsDocs.js`:
- Around line 1-18: Add JSDoc for the exported QueryParamsArgumentsDocs function
so it complies with the JS/TS/JSX coding guideline: document the queryParams
input with an `@param` tag and describe the return value with `@returns`. Keep the
docs attached directly above QueryParamsArgumentsDocs, and ensure they cover the
component’s behavior when queryParams is empty or missing.
---
Nitpick comments:
In
`@packages/templates/clients/websocket/javascript/test/components/InitSignature.test.js`:
- Around line 28-38: Add a test case in InitSignature.test.js to cover a query
param entry without a default value, since InitSignature’s defaultValue handling
currently only has coverage through the truthy default branches and the
multi-param fixture. Extend the existing render snapshot tests for InitSignature
so one case passes a param tuple with no second element and asserts the rendered
output, exercising the paramDefaultValue ? ... : '' path.
- Around line 1-16: The parser setup in InitSignature.test.js is duplicated from
QueryParamsArgumentsDocs.test.js, so extract the shared AsyncAPI document
loading logic into a common test helper. Move the repeated Parser/fromFile
fixture path and beforeAll parsing flow into a reusable utility, then have
InitSignature and QueryParamsArgumentsDocs import and use it so the parsed
document is created in one place.
🪄 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: 70315b57-c37f-48a9-ac43-41e9e7992f6c
⛔ Files ignored due to path filters (4)
packages/components/test/components/__snapshots__/QueryParamsVariables.test.js.snapis excluded by!**/*.snappackages/templates/clients/websocket/javascript/test/components/__snapshots__/InitSignature.test.js.snapis excluded by!**/*.snappackages/templates/clients/websocket/javascript/test/components/__snapshots__/QueryParamsArgumentsDocs.test.js.snapis excluded by!**/*.snappackages/templates/clients/websocket/test/integration-test/__snapshots__/integration.test.js.javascript.snapis excluded by!**/*.snap
📒 Files selected for processing (8)
packages/components/src/components/QueryParamsVariables.jspackages/templates/clients/websocket/javascript/components/ClientClass.jspackages/templates/clients/websocket/javascript/components/Constructor.jspackages/templates/clients/websocket/javascript/components/InitSignature.jspackages/templates/clients/websocket/javascript/components/QueryParamsArgumentsDocs.jspackages/templates/clients/websocket/javascript/template/client.js.jspackages/templates/clients/websocket/javascript/test/components/InitSignature.test.jspackages/templates/clients/websocket/javascript/test/components/QueryParamsArgumentsDocs.test.js
@Adi-204 I wonder if we can actually put the Common JS file for parsing the AsyncAPI document, because in all other tests we don't follow that way 🤔 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/templates/clients/websocket/javascript/components/getSafeJsName.js (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStray formatting inside template literal.
`_${ safe}`has an unnecessary double space inside the expression. As per coding guidelines, formatting should be enforced entirely through ESLint rather than manual/Prettier-style edits; run lint to normalize this.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/templates/clients/websocket/javascript/components/getSafeJsName.js` at line 18, The template literal in getSafeJsName contains stray manual spacing inside the interpolation, so normalize this through the existing lint/formatting rules rather than hand-editing the expression. Update the safe-name fallback in getSafeJsName to follow the repository’s ESLint style, and verify the change by running the relevant lint fix for the JavaScript template component.Source: Coding guidelines
packages/templates/clients/websocket/javascript/test/components/InitSignature.test.js (1)
1-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffShared parser/fixture setup duplicated across test files.
This mirrors the duplication already raised in the PR discussion about extracting a shared helper for loading the parsed AsyncAPI document. Worth consolidating once the placement question (CommonJS parsing file vs. existing test patterns) is resolved.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/templates/clients/websocket/javascript/test/components/InitSignature.test.js` around lines 1 - 16, The AsyncAPI document loading setup in InitSignature.test.js is duplicated across multiple test files, so extract the shared parser/fixture initialization into a reusable helper. Move the repeated Parser, fromFile, asyncapiFilePath, and beforeAll parse logic into the agreed shared location, then update InitSignature and the other affected component tests to consume that helper instead of repeating the setup.packages/templates/clients/websocket/javascript/test/components/getSafeJsName.test.js (1)
3-35: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider adding a collision test once dedup logic is added.
Given the identifier-collision risk flagged in
getSafeJsName.js(distinct names likemy-param/my_paramboth sanitizing tomyParam), it would be worth adding a test case covering that scenario once the helper handles it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/templates/clients/websocket/javascript/test/components/getSafeJsName.test.js` around lines 3 - 35, Add a new collision-focused test in getSafeJsName.test.js for the getSafeJSName helper to cover distinct inputs that currently sanitize to the same output, such as my-param and my_param both becoming myParam. Once the dedup logic is implemented in getSafeJsName.js, assert that the helper returns unique, non-conflicting names for colliding identifiers and keep the test alongside the existing conversion and reserved-word cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/templates/clients/websocket/javascript/components/getSafeJsName.js`:
- Around line 11-19: The sanitization helper in getSafeJSName can return the
same identifier for different original names, which leads to duplicate JS
parameter names downstream. Update getSafeJSName (or the callers
InitSignature.js and QueryParamsArgumentsDocs.js) to track already-used names
and generate a unique fallback, such as appending a numeric suffix when a
collision is detected. Keep the existing reserved-word and leading-digit
handling, but ensure each returned name is distinct within the generated
parameter list.
---
Nitpick comments:
In `@packages/templates/clients/websocket/javascript/components/getSafeJsName.js`:
- Line 18: The template literal in getSafeJsName contains stray manual spacing
inside the interpolation, so normalize this through the existing lint/formatting
rules rather than hand-editing the expression. Update the safe-name fallback in
getSafeJsName to follow the repository’s ESLint style, and verify the change by
running the relevant lint fix for the JavaScript template component.
In
`@packages/templates/clients/websocket/javascript/test/components/getSafeJsName.test.js`:
- Around line 3-35: Add a new collision-focused test in getSafeJsName.test.js
for the getSafeJSName helper to cover distinct inputs that currently sanitize to
the same output, such as my-param and my_param both becoming myParam. Once the
dedup logic is implemented in getSafeJsName.js, assert that the helper returns
unique, non-conflicting names for colliding identifiers and keep the test
alongside the existing conversion and reserved-word cases.
In
`@packages/templates/clients/websocket/javascript/test/components/InitSignature.test.js`:
- Around line 1-16: The AsyncAPI document loading setup in InitSignature.test.js
is duplicated across multiple test files, so extract the shared parser/fixture
initialization into a reusable helper. Move the repeated Parser, fromFile,
asyncapiFilePath, and beforeAll parse logic into the agreed shared location,
then update InitSignature and the other affected component tests to consume that
helper instead of repeating the setup.
🪄 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: 71c93031-8a1a-4115-b34e-282d30263bf3
⛔ Files ignored due to path filters (1)
packages/templates/clients/websocket/javascript/test/components/__snapshots__/InitSignature.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
packages/templates/clients/websocket/javascript/components/InitSignature.jspackages/templates/clients/websocket/javascript/components/QueryParamsArgumentsDocs.jspackages/templates/clients/websocket/javascript/components/getSafeJsName.jspackages/templates/clients/websocket/javascript/test/components/InitSignature.test.jspackages/templates/clients/websocket/javascript/test/components/getSafeJsName.test.js
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/templates/clients/websocket/javascript/components/QueryParamsArgumentsDocs.js
- packages/templates/clients/websocket/javascript/components/InitSignature.js
|
| usedNames.add(candidate); | ||
|
|
||
| return candidate; | ||
| } No newline at end of file |
There was a problem hiding this comment.
Firstly I don't get point of this getSafeJsName becoz we can't just change param name given in the input asyncapi file. Secondly it is not a component it should be in file https://github.com/asyncapi/generator/blob/master/packages/helpers/src/utils.js
There was a problem hiding this comment.
The main use of getSafeJsName is, if the paramName that the user has sent is, for example, user-id, as it is not valid js variable name it would give a syntax error so to cut that off we are basically using this function
Sure thing, I should have put this in the utils.js.
Also, since we can't change the paramName in the AsyncAPI doc, what if paramName is invalid? What can we actually do here??
|
I have tested the code using this file. |
|
hey @batchu5 we discuss in the generator meeting https://fathom.video/share/P7tie39Wzz6zzHK3VuY-kkBh8nbR_jBX you can have a look at it basically right now it is a bit difficult to isolate this adding a new component to a template as we are not able to test it properly I think better would be changing the scope of issue and you can continue in this PR adding support for slack client in JS. NO need to complicate it too much for Phase 1 we don't want everything that python is doing right now rather main objective is to have slack support in JS client. Feel free to ask question if any. |
|
@batchu5 new scope of the issue - "basic version of working slack example" If you notice in python template https://github.com/asyncapi/generator/tree/master/packages/templates/clients/websocket/python we have 3 examples example.py, example-slack.py and example-slack-with-routing.py Now to keep scope limited you only need to make sure for JS template something like example-slack.py is working which was introduce in #1509 What you DON"T need to DO - "automatic routing of message for slack" which was introduce in #1814 keep this OUT OF SCOPE. Feel free to ask question! |
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/templates/clients/websocket/javascript/example-slack.js (1)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace descriptive comments with required JSDoc.
Remove the comment at Line 2. Replace the comment at Line 5 with JSDoc for
myHandler. Add JSDoc formainwith the parameter type, return type, and error behavior. Document that connection errors are caught and logged, and that the pending promise remains active while the client listens.As per coding guidelines, comments should explain non-obvious why factors, and JavaScript functions require clear JSDoc with parameter types, return values, and error conditions.
Proposed documentation
-// Example usage const wsClient = new WSClient(); -// Example of how custom message handler that operates on incoming messages can look like +/** + * Logs an incoming Slack event. + * `@param` {unknown} message Incoming Slack event. + * `@returns` {void} + */ function myHandler(message) { ... +/** + * Registers the handler, connects to Slack, and listens for events. + * `@returns` {Promise<void>} A promise that remains pending while the client listens. + * `@throws` {Error} If message-handler registration fails. + */ async function main() {Also applies to: 5-6, 12-12
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/templates/clients/websocket/javascript/example-slack.js` at line 2, Remove the standalone descriptive comment, replace the existing comment above myHandler with JSDoc documenting its parameters and return value, and add JSDoc above main covering its parameter type, return type, connection-error handling and logging, and the pending promise remaining active while the client listens.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/components/src/components/QueryParamsVariables.js`:
- Around line 68-80: Update QueryParamsVariables to reuse InitSignature’s
getSafeJSName mapping for generated local identifiers, applying safe names
consistently in declarations, conditions, and assignments. Generate environment
lookups with process.env[JSON.stringify(rawName)] and query keys with
JSON.stringify(rawName), then add regression coverage for non-identifier,
reserved, and colliding parameter names.
In `@packages/templates/clients/websocket/javascript/example-slack.js`:
- Line 3: Update the WSClient initialization in the example to pass the
connection URL returned by apps.connections.open, using new WSClient(url) so the
required ticket and app_id query parameters are preserved.
---
Nitpick comments:
In `@packages/templates/clients/websocket/javascript/example-slack.js`:
- Line 2: Remove the standalone descriptive comment, replace the existing
comment above myHandler with JSDoc documenting its parameters and return value,
and add JSDoc above main covering its parameter type, return type,
connection-error handling and logging, and the pending promise remaining active
while the client listens.
🪄 Autofix
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 Plus
Run ID: 73cdeb3c-b34b-429a-a54c-5694d1c7afcb
⛔ Files ignored due to path filters (2)
packages/components/test/components/__snapshots__/QueryParamsVariables.test.js.snapis excluded by!**/*.snappackages/templates/clients/websocket/test/integration-test/__snapshots__/integration.test.js.javascript.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
packages/components/src/components/QueryParamsVariables.jspackages/templates/clients/websocket/javascript/components/getSafeJsName.jspackages/templates/clients/websocket/javascript/example-slack.jspackages/templates/clients/websocket/test/integration-test/integration.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/templates/clients/websocket/javascript/components/getSafeJsName.js
| const rawName = param[0]; | ||
| const paramName = toCamelCase(rawName); | ||
| return { | ||
| variableDefinition: { | ||
| text: `const ${paramName} = ${paramName} || process.env.${paramName.toUpperCase()};`, | ||
| indent: 8, | ||
| text: `const _${rawName} = ${paramName} || process.env.${rawName.toUpperCase()};`, | ||
| indent: 0, | ||
| }, | ||
| ifCondition: { | ||
| text: `if (${paramName}) {`, | ||
| indent: 8, | ||
| text: `if (_${rawName}) {`, | ||
| indent: 0, | ||
| }, | ||
| assignment: { | ||
| text: `params["${paramName}"] = ${paramName};`, | ||
| indent: 10, | ||
| text: `params["${rawName}"] = _${rawName};`, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 '\b(toCamelCase|getSafeJsName)\b' \
packages/components/src/components/QueryParamsVariables.js \
packages/templates/clients/websocket/javascript/components
rg -n -C 3 'auth-token|foo-bar|queryParams' \
packages/components/test \
packages/templates/clients/websocket/javascript/testRepository: asyncapi/generator
Length of output: 28865
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- QueryParamsVariables.js ---'
cat -n packages/components/src/components/QueryParamsVariables.js
printf '%s\n' '--- getSafeJsName.js ---'
cat -n packages/templates/clients/websocket/javascript/components/getSafeJsName.js
printf '%s\n' '--- InitSignature.js ---'
cat -n packages/templates/clients/websocket/javascript/components/InitSignature.js
printf '%s\n' '--- component tests ---'
cat -n packages/components/test/components/QueryParamsVariables.test.js
printf '%s\n' '--- JavaScript snapshots ---'
rg -n -A18 -B3 'javascript' packages/components/test/components/__snapshots__/QueryParamsVariables.test.js.snap
printf '%s\n' '--- QueryParamsVariables usages ---'
rg -n -C3 '<QueryParamsVariables|QueryParamsVariables' packages --glob '*.{js,jsx,ts,tsx}'Repository: asyncapi/generator
Length of output: 16195
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- helper usages and exports ---'
rg -n -C3 'getSafeJSName|QueryParamsVariables|QueryParamsArgumentsDocs|InitSignature' \
packages/components packages/templates/clients/websocket/javascript
printf '%s\n' '--- JavaScript snapshot section ---'
sed -n '1,25p' packages/components/test/components/__snapshots__/QueryParamsVariables.test.js.snap
printf '%s\n' '--- helper source candidates ---'
rg -n -C5 'function toCamelCase|const toCamelCase|export .*toCamelCase' \
packages
printf '%s\n' '--- package README principles ---'
sed -n '1,220p' packages/README.md
printf '%s\n' '--- deterministic safe-name probe ---'
python3 - <<'PY'
import re
def to_camel_case(name):
# Probe the cases relevant to this review using the repository's helper
# implementation if it is available as a readable source file.
return name
def safe_js_name(name):
safe = to_camel_case(name)
safe = re.sub(r'[^a-zA-Z0-9_]', '_', safe)
if re.match(r'^[0-9]', safe):
safe = '_' + safe
if safe in {
'url', 'throwSendErrors', 'params', 'queryString',
'class', 'const', 'let', 'var', 'if', 'else', 'return',
'this', 'true', 'false', 'null', 'undefined',
}:
safe = '_' + safe
return safe
for raw in ['auth-token', 'foo-bar', '1token', 'class', 'params', 'token']:
print(raw, 'raw identifier:', f'const _{raw}', 'safe helper:', safe_js_name(raw))
PYRepository: asyncapi/generator
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- toCamelCase implementation ---'
rg -l 'toCamelCase' packages | while read -r file; do
rg -n -C8 'toCamelCase' "$file"
done | head -n 180
printf '%s\n' '--- focused constructor composition ---'
cat -n packages/templates/clients/websocket/javascript/components/Constructor.js
printf '%s\n' '--- focused safe-name tests ---'
cat -n packages/templates/clients/websocket/javascript/test/components/getSafeJsName.test.js
printf '%s\n' '--- JavaScript syntax probe ---'
node - <<'JS'
const cases = [
{
name: 'auth-token',
argument: 'authToken',
raw: 'const _auth-token = authToken || process.env.AUTH-TOKEN;',
corrected: 'const _authToken = authToken || process.env["AUTH-TOKEN"];',
},
{
name: 'url',
argument: '_url',
raw: 'const _url = url || process.env.URL;',
corrected: 'const __url = _url || process.env["URL"];',
},
{
name: 'throw-send-errors',
argument: '_throwSendErrors',
raw: 'const _throw-send-errors = throwSendErrors || process.env.THROW-SEND-ERRORS;',
corrected: 'const __throwSendErrors = _throwSendErrors || process.env["THROW-SEND-ERRORS"];',
},
];
for (const item of cases) {
for (const [label, source] of [['raw', item.raw], ['corrected', item.corrected]]) {
try {
new Function(`function f(${item.argument}) { ${source} }`);
console.log(item.name, label, 'PARSES');
} catch (error) {
console.log(item.name, label, 'REJECTED:', error.message);
}
}
}
JSRepository: asyncapi/generator
Length of output: 11961
Use safe constructor names in generated query code.
QueryParamsVariables must use the same getSafeJSName mapping as InitSignature. Names such as auth-token, 1st, and url can otherwise produce invalid or duplicate declarations. Use the safe name for all local references, process.env[JSON.stringify(...)] for environment variables, and JSON.stringify(rawName) for query keys. Add regression tests for non-identifier, reserved, and colliding names.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/components/src/components/QueryParamsVariables.js` around lines 68 -
80, Update QueryParamsVariables to reuse InitSignature’s getSafeJSName mapping
for generated local identifiers, applying safe names consistently in
declarations, conditions, and assignments. Generate environment lookups with
process.env[JSON.stringify(rawName)] and query keys with
JSON.stringify(rawName), then add regression coverage for non-identifier,
reserved, and colliding parameter names.
| @@ -0,0 +1,27 @@ | |||
| const WSClient = require('./test/temp/snapshotTestResult/client_slack/client.js'); | |||
| // Example usage | |||
| const wsClient = new WSClient(); | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 'new WSClient|constructor\(|query|ticket|app_id' \
packages/templates/clients/websocket/javascript \
packages/templates/clients/websocket/testRepository: asyncapi/generator
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- tracked Slack-related files ---'
git ls-files packages/templates/clients/websocket | grep -Ei 'slack|fixture|snapshot' | head -80
echo '--- focused generated-client references ---'
rg -n -C 5 'class Slack|new WSClient|wss-primary\.slack\.com/link|ticket.*app_id|app_id.*ticket' \
packages/templates/clients/websocket --glob '!**/*.python.snap' --glob '!**/node_modules/**' | head -240
echo '--- constructor generation ---'
sed -n '1,90p' packages/templates/clients/websocket/javascript/components/InitSignature.js
sed -n '1,90p' packages/templates/clients/websocket/javascript/components/Constructor.js
echo '--- example ---'
cat -n packages/templates/clients/websocket/javascript/example-slack.jsRepository: asyncapi/generator
Length of output: 27822
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Slack fixture ---'
cat -n packages/templates/clients/websocket/test/__fixtures__/asyncapi-slack-client.yml | sed -n '1,90p'
echo '--- generated JavaScript constructor snapshot ---'
sed -n '1106,1165p' packages/templates/clients/websocket/test/integration-test/__snapshots__/integration.test.js.javascript.snap
echo '--- JavaScript Slack documentation and example references ---'
find packages/templates/clients/websocket/javascript -maxdepth 2 -type f -print | sort
rg -n -C 8 'apps\.connections\.open|ticket|app_id|example-slack|client_slack' \
packages/templates/clients/websocket/javascript \
packages/templates/clients/websocket/test/__fixtures__/asyncapi-slack-client.yml \
packages/templates/clients/websocket/test/integration-test/__snapshots__/integration.test.js.javascript.snapRepository: asyncapi/generator
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
from pathlib import Path
fixture = Path("packages/templates/clients/websocket/test/__fixtures__/asyncapi-slack-client.yml").read_text()
snapshot = Path(
"packages/templates/clients/websocket/test/integration-test/"
"__snapshots__/integration.test.js.javascript.snap"
).read_text()
fixture_section = fixture.split("properties:", 1)[1].split("operations:", 1)[0]
assert re.search(r"^\s+ticket:\s*$", fixture_section, re.MULTILINE)
assert re.search(r"^\s+app_id:\s*$", fixture_section, re.MULTILINE)
constructor = re.search(
r"constructor\(([^)]*)\) \{(?P<body>.*?)\n \}",
snapshot,
re.DOTALL,
)
assert constructor, "Slack constructor not found"
signature = constructor.group(1)
body = constructor.group("body")
assert signature == "url, ticket, appId, throwSendErrors = true"
assert "ticket = " not in signature
assert "appId = " not in signature
assert "this.url = url || 'wss://wss-primary.slack.com/link';" in body
assert "params[\\\\\"ticket\\\\\"] = _ticket;" in body
assert "params[\\\\\"app_id\\\\\"] = _app_id;" in body
# Model the constructor's zero-argument path without executing repository code.
url = "wss://wss-primary.slack.com/link"
ticket = None
app_id = None
if ticket or None:
url += "?ticket=" + ticket
if app_id or None:
url += ("&" if "?" in url else "?") + "app_id=" + app_id
print("constructor signature:", signature)
print("zero-argument URL:", url)
print("zero-argument URL has ticket:", "ticket=" in url)
print("zero-argument URL has app_id:", "app_id=" in url)
assert url == "wss://wss-primary.slack.com/link"
PYRepository: asyncapi/generator
Length of output: 346
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
from pathlib import Path
fixture = Path("packages/templates/clients/websocket/test/__fixtures__/asyncapi-slack-client.yml").read_text()
snapshot = Path(
"packages/templates/clients/websocket/test/integration-test/"
"__snapshots__/integration.test.js.javascript.snap"
).read_text()
fixture_section = fixture.split("properties:", 1)[1].split("operations:", 1)[0]
assert re.search(r"^\s+ticket:\s*$", fixture_section, re.MULTILINE)
assert re.search(r"^\s+app_id:\s*$", fixture_section, re.MULTILINE)
constructor = re.search(
r"constructor\(([^)]*)\) \{(?P<body>.*?)\n \}",
snapshot,
re.DOTALL,
)
assert constructor, "Slack constructor not found"
signature = constructor.group(1)
body = constructor.group("body")
assert signature == "url, ticket, appId, throwSendErrors = true"
assert "ticket = " not in signature
assert "appId = " not in signature
assert "this.url = url || 'wss://wss-primary.slack.com/link';" in body
assert "params[\\\\\"ticket\\\\\"] = _ticket;" in body
assert "params[\\\\\"app_id\\\\\"] = _app_id;" in body
url = "wss://wss-primary.slack.com/link"
ticket = None
app_id = None
if ticket or None:
url += "?ticket=" + ticket
if app_id or None:
url += ("&" if "?" in url else "?") + "app_id=" + app_id
print("constructor signature:", signature)
print("zero-argument URL:", url)
print("zero-argument URL has ticket:", "ticket=" in url)
print("zero-argument URL has app_id:", "app_id=" in url)
assert url == "wss://wss-primary.slack.com/link"
PYRepository: asyncapi/generator
Length of output: 346
Pass Slack’s generated connection URL to WSClient.
new WSClient() omits the required ticket and app_id query parameters. Pass the url returned by apps.connections.open to new WSClient(url).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/templates/clients/websocket/javascript/example-slack.js` at line 3,
Update the WSClient initialization in the example to pass the connection URL
returned by apps.connections.open, using new WSClient(url) so the required
ticket and app_id query parameters are preserved.
3 participants
Description
This PR adds support for query parameters to the generated JavaScript WebSocket client, bringing it closer to feature parity with the Python and Java clients.
Changes Made
packages/templates/clients/websocket/javascript):ClientClass.jsandConstructor.jsto process and append query parameters to the WebSocket URL using the native Node.jsquerystringmodule.QueryParamsArgumentsDocs.jsto dynamically generate JSDoc@paramdescriptions for each query parameter.InitSignature.jsto inject query parameters into the constructor signature, defaulting to the values specified in the AsyncAPI document.querystringmodule dependency only whenqueryParamsare present in the AsyncAPI document.Generated-by: Claude Opus 4.6
Fixes #1957
Summary by CodeRabbit
New Features
Bug Fixes
Tests