feat: release @asyncapi/keeper and use for message validation in JS websocket client
#1747
Conversation
Adi-204
requested review from
asyncapi-bot-eve,
derberg,
jonaslagoni and
magicmatatjahu
as code owners
🦋 Changeset detectedLatest commit: b16dbb4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
|
WalkthroughReplaces Hyperjump JSON Schema with AJV in the keeper app, removes structuredClone polyfill from Jest setup, centralizes Jest configuration into a shared Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 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)
⏰ 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). (4)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/components/src/components/SendOperations.js (1)
74-99: Critical: Validation logic has multiple issues.The validation implementation has several problems:
Line 77: Early return
{ isValid: true }when no schemas provided bypasses validation silently. This should at least log a warning or be explicitly documented as intended behavior.Lines 92-95: The
if (!isValid)check is inside theforloop, so it executes on every iteration. It should be moved outside the loop to check the final validation result.Logic flaw: When validation fails, the code logs errors but doesn't prevent the message from being sent (if it was sent in a previous iteration) and doesn't throw an error as documented in the JSDoc (@throws).
Missing final validation check: After the loop, if
!isValid, the function should throw an error rather than silently returning.Here's a corrected implementation:
static async ${methodName}(message, socket, schemas) { try { if (!schemas || schemas.length === 0) { - return { isValid: true }; + console.warn('No schemas provided for validation. Skipping validation.'); + socket.send(JSON.stringify(message)); + return; } const allValidationErrors = []; let isValid = false; for(const compiledSchema of schemas){ const validationResult = validateMessage(compiledSchema, message); if (validationResult.isValid) { isValid = true; socket.send(JSON.stringify(message)); - break; + return; // Successfully validated and sent } else { if (validationResult.validationErrors) { allValidationErrors.push(...validationResult.validationErrors); } } - if (!isValid) { - console.error('Validation errors:', JSON.stringify(allValidationErrors, null, 2)); - } } + // If we reach here, validation failed against all schemas + console.error('Validation errors:', JSON.stringify(allValidationErrors, null, 2)); + throw new Error(\`Message validation failed for ${methodName}: \${JSON.stringify(allValidationErrors)}\`); } catch (error) { console.error('Error sending ${methodName} message:', error); + throw error; // Re-throw to match JSDoc } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
package-lock.jsonis excluded by!**/package-lock.jsonpackages/components/test/components/__snapshots__/DependencyProvider.test.js.snapis excluded by!**/*.snappackages/components/test/components/__snapshots__/SendOperations.test.js.snapis excluded by!**/*.snappackages/templates/clients/websocket/test/integration-test/__snapshots__/integration.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (15)
apps/keeper/jest.setup.js(0 hunks)apps/keeper/package.json(1 hunks)apps/keeper/src/index.js(2 hunks)apps/keeper/src/utils.js(0 hunks)apps/keeper/test/__fixtures__/asyncapi-message-validation.yml(1 hunks)apps/keeper/test/index.test.js(2 hunks)packages/components/src/components/DependencyProvider.js(1 hunks)packages/components/src/components/SendOperations.js(1 hunks)packages/templates/clients/websocket/javascript/components/ClientClass.js(2 hunks)packages/templates/clients/websocket/javascript/components/CompileOperationSchemas.js(1 hunks)packages/templates/clients/websocket/javascript/components/Constructor.js(2 hunks)packages/templates/clients/websocket/javascript/example.js(2 hunks)packages/templates/clients/websocket/javascript/package.json(1 hunks)packages/templates/clients/websocket/javascript/template/client.js.js(1 hunks)packages/templates/clients/websocket/test/__fixtures__/asyncapi-postman-echo.yml(1 hunks)
💤 Files with no reviewable changes (2)
- apps/keeper/src/utils.js
- apps/keeper/jest.setup.js
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-05-12T14:23:48.919Z
Learnt from: derberg
Repo: asyncapi/generator PR: 1551
File: apps/generator/docs/generator-template.md:45-73
Timestamp: 2025-05-12T14:23:48.919Z
Learning: AsyncAPI 3.0.0 specification introduces significant structural changes from 2.x:
1. Operations become top-level elements with action property (send/receive) and references to channels
2. Channels use 'address' for the topic path instead of having nested publish/subscribe operations
3. Messages are defined under a 'messages' container in channels
4. The specification decouples operations, channels and messages to improve reusability
5. Servers can use a 'host' property
Applied to files:
packages/templates/clients/websocket/test/__fixtures__/asyncapi-postman-echo.ymlpackages/templates/clients/websocket/javascript/components/Constructor.jsapps/keeper/test/__fixtures__/asyncapi-message-validation.ymlpackages/components/src/components/SendOperations.js
📚 Learning: 2025-05-12T14:23:48.919Z
Learnt from: derberg
Repo: asyncapi/generator PR: 1551
File: apps/generator/docs/generator-template.md:45-73
Timestamp: 2025-05-12T14:23:48.919Z
Learning: AsyncAPI 3.0.0 specification introduces significant structural changes from 2.x:
1. Operations become top-level elements with references to channels
2. Channels use 'address' for the topic path instead of having nested publish/subscribe
3. Messages are defined under a 'messages' container in channels
4. Servers can use a 'host' property
Applied to files:
packages/templates/clients/websocket/test/__fixtures__/asyncapi-postman-echo.ymlapps/keeper/test/__fixtures__/asyncapi-message-validation.ymlpackages/components/src/components/SendOperations.js
📚 Learning: 2025-05-12T14:57:35.024Z
Learnt from: Adi-204
Repo: asyncapi/generator PR: 1557
File: packages/templates/clients/websocket/javascript/components/SendOperation.js:21-21
Timestamp: 2025-05-12T14:57:35.024Z
Learning: In the AsyncAPI generator templates, particularly in the JavaScript WebSocket client, single quotes are used for string literals inside template-generated code (which is itself inside backticks) to preserve template placeholders like ${variable} as literal text in the generated code output.
Applied to files:
packages/templates/clients/websocket/javascript/template/client.js.jspackages/templates/clients/websocket/javascript/example.jspackages/templates/clients/websocket/javascript/package.json
📚 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/templates/clients/websocket/javascript/components/CompileOperationSchemas.js
🧬 Code graph analysis (8)
packages/templates/clients/websocket/javascript/components/Constructor.js (2)
packages/templates/clients/websocket/javascript/template/client.js.js (1)
sendOperations(12-12)packages/templates/clients/websocket/python/components/ClientClass.js (1)
sendOperations(16-16)
packages/templates/clients/websocket/javascript/template/client.js.js (1)
packages/components/src/components/DependencyProvider.js (1)
DependencyProvider(106-116)
packages/templates/clients/websocket/javascript/components/ClientClass.js (2)
packages/templates/clients/websocket/javascript/components/Constructor.js (1)
Constructor(3-28)packages/templates/clients/websocket/javascript/components/CompileOperationSchemas.js (1)
CompileOperationSchemas(3-40)
apps/keeper/test/index.test.js (1)
apps/keeper/src/index.js (5)
compileSchema(12-14)validateMessage(70-85)compiledSchemas(26-26)compileSchemas(22-31)compileSchemasByOperationId(41-59)
apps/keeper/src/index.js (2)
apps/generator/lib/templates/config/validator.js (1)
Ajv(2-2)apps/keeper/src/utils.js (3)
asyncapi(16-16)parseAsyncAPIDocumentFromFile(12-24)parseAsyncAPIDocumentFromFile(12-24)
packages/components/src/components/SendOperations.js (2)
packages/templates/clients/websocket/javascript/template/client.js.js (1)
clientName(10-10)packages/templates/clients/websocket/python/components/ClientClass.js (1)
clientName(13-13)
packages/templates/clients/websocket/javascript/example.js (1)
packages/templates/clients/websocket/test/javascript/acceptance.test.js (2)
WSClient(5-5)require(6-6)
packages/templates/clients/websocket/javascript/components/CompileOperationSchemas.js (1)
packages/templates/clients/websocket/javascript/template/client.js.js (1)
sendOperations(12-12)
⏰ 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 - ubuntu-latest
- GitHub Check: Test NodeJS PR - macos-13
🔇 Additional comments (6)
packages/templates/clients/websocket/javascript/example.js (1)
26-28: Change verified as correct.The message format
{ content: 'Hello!' }correctly matches thesendEchoMessageschema defined in the AsyncAPI 3.0.0 fixture. The object includes the requiredcontentproperty (string type) and properly omits the optionalmessageIdfield. This structure aligns with the schema validation approach and will pass validation.apps/keeper/test/__fixtures__/asyncapi-message-validation.yml (1)
34-36: LGTM!The schema update from
timestamptomessageIdis clean and appropriate for test fixture usage.packages/templates/clients/websocket/javascript/components/ClientClass.js (1)
5-5: LGTM!The integration of
CompileOperationSchemasis properly structured, with correct imports, prop passing, and render order (beforeSendOperations).Also applies to: 13-13, 27-27
packages/templates/clients/websocket/javascript/components/Constructor.js (1)
3-6: LGTM!The constructor properly initializes the schema compilation state (
compiledSchemas,schemasCompiled,sendOperationsId), enabling per-operation schema handling.Also applies to: 20-22
apps/keeper/test/index.test.js (1)
4-4: LGTM!Test suite properly updated to use the new API (
compileSchema,compileSchemas,validateMessage). Good coverage of validation scenarios, error cases, and operation-specific validation.Also applies to: 15-23, 25-90, 92-129
packages/templates/clients/websocket/javascript/components/CompileOperationSchemas.js (1)
3-39: Component structure is sound, but depends on hardcoded path.The generated
compileOperationSchemas()method is well-structured with proper guards (schemasCompiledcheck) and error handling. However, it referencesasyncapiFilepathwhich is currently hardcoded to a test fixture path inclient.js.js(see my comment on that file).Once the
asyncapiFilepathissue is resolved, this component will function correctly.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/keeper/src/index.js (1)
70-85: Update error message to reflect that null is allowed.The validation logic correctly only checks for
undefined(notnull), allowing schemas that permitnullpayloads to work as intended. However, the error message on line 72 still mentions "non-null object", which is misleading.Apply this diff to clarify the error message:
- throw new Error(`Invalid "message" parameter: expected a non-null object to validate, but received ${message}`); + throw new Error(`Invalid "message" parameter: expected a value to validate, but received ${message}`);This aligns with the suggested fix from the previous review comment and accurately reflects that
nullis now a valid input for validation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
package-lock.jsonis excluded by!**/package-lock.jsonpackages/components/test/components/__snapshots__/SendOperations.test.js.snapis excluded by!**/*.snappackages/templates/clients/websocket/test/integration-test/__snapshots__/integration.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
apps/keeper/package.json(1 hunks)apps/keeper/src/index.js(2 hunks)apps/keeper/test/index.test.js(2 hunks)packages/components/src/components/SendOperations.js(1 hunks)packages/templates/clients/websocket/javascript/components/Constructor.js(2 hunks)packages/templates/clients/websocket/javascript/example.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/templates/clients/websocket/javascript/example.js
- packages/templates/clients/websocket/javascript/components/Constructor.js
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-12T14:23:48.919Z
Learnt from: derberg
Repo: asyncapi/generator PR: 1551
File: apps/generator/docs/generator-template.md:45-73
Timestamp: 2025-05-12T14:23:48.919Z
Learning: AsyncAPI 3.0.0 specification introduces significant structural changes from 2.x:
1. Operations become top-level elements with action property (send/receive) and references to channels
2. Channels use 'address' for the topic path instead of having nested publish/subscribe operations
3. Messages are defined under a 'messages' container in channels
4. The specification decouples operations, channels and messages to improve reusability
5. Servers can use a 'host' property
Applied to files:
packages/components/src/components/SendOperations.js
📚 Learning: 2025-05-12T14:23:48.919Z
Learnt from: derberg
Repo: asyncapi/generator PR: 1551
File: apps/generator/docs/generator-template.md:45-73
Timestamp: 2025-05-12T14:23:48.919Z
Learning: AsyncAPI 3.0.0 specification introduces significant structural changes from 2.x:
1. Operations become top-level elements with references to channels
2. Channels use 'address' for the topic path instead of having nested publish/subscribe
3. Messages are defined under a 'messages' container in channels
4. Servers can use a 'host' property
Applied to files:
packages/components/src/components/SendOperations.js
🧬 Code graph analysis (3)
apps/keeper/test/index.test.js (2)
apps/keeper/src/utils.js (2)
parser(2-2)parseResult(18-18)apps/keeper/src/index.js (5)
compileSchema(12-14)validateMessage(70-85)compiledSchemas(26-26)compileSchemas(22-31)compileSchemasByOperationId(41-59)
packages/components/src/components/SendOperations.js (2)
packages/templates/clients/websocket/javascript/template/client.js.js (1)
clientName(10-10)packages/templates/clients/websocket/python/components/ClientClass.js (1)
clientName(13-13)
apps/keeper/src/index.js (2)
apps/generator/lib/templates/config/validator.js (1)
Ajv(2-2)apps/keeper/src/utils.js (3)
asyncapi(16-16)parseAsyncAPIDocumentFromFile(12-24)parseAsyncAPIDocumentFromFile(12-24)
⏰ 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 18
- GitHub Check: Acceptance tests for generated templates
- GitHub Check: Test generator as dependency with Node 20
- GitHub Check: Test NodeJS PR - macos-13
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - ubuntu-latest
🔇 Additional comments (11)
apps/keeper/package.json (3)
21-21: AJV dependency addition aligns with migration goals.The addition of
ajvfor JSON schema validation is correct and directly supports the PR objective to migrate from Hyperjump JSON Schema to AJV-based validation.
27-27: Critical issue resolved: babel-jest dependency added.The previous review flagged that
babel-jestwas referenced in Jest's transform config but missing from devDependencies. This has now been correctly added at version^27.3.1, which aligns with the installed Jest version and should be compatible.
38-42: Module mappings correctly updated for dependency migration.The moduleNameMapper updates appropriately remove mappings for the deprecated Hyperjump packages and retain only the necessary nimma mappings, reflecting the migration from
@hyperjump/json-schematoajv.apps/keeper/test/index.test.js (4)
13-17: LGTM! Helper simplifies test setup.The
parseAsyncAPIFilehelper centralizes the parsing logic and makes tests cleaner.
19-71: LGTM! Comprehensive coverage of basic validation scenarios.The tests properly exercise the new single-schema validation flow, including success cases, validation failures, and error conditions.
73-90: LGTM! Good coverage for compileSchemas.The tests validate both the happy path (multiple schema compilation) and error handling (invalid parameter type).
92-129: LGTM! Thorough coverage of operation-specific validation.The tests properly exercise
compileSchemasByOperationIdincluding error cases for invalid and empty operation IDs.apps/keeper/src/index.js (4)
1-4: LGTM! Appropriate AJV configuration.The AJV instance is configured with
strict: falsefor flexibility andallErrors: trueto capture all validation errors.
6-14: LGTM! Clean wrapper for AJV compilation.The function properly delegates to AJV's compile method and returns a validator function.
16-31: LGTM! Proper input validation and delegation.The function validates the input parameter type and correctly delegates to
compileSchemafor each schema.
33-59: LGTM! Enhanced validation and error handling.The function now includes stronger input validation, proper operation existence checks, and gracefully handles missing messages with a warning.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/components/src/components/SendOperations.js (1)
74-100: Fix validation logic bugs and error handling.The static method has several critical issues:
Logic bug (lines 93-95): The
if (!isValid)check is inside the for loop, so validation errors are logged on every failed schema instead of once after exhausting all schemas. Move this check outside the loop.Missing throw on validation failure: The JSDoc promises
@throws {Error} If message validation fails against all schemas(line 72), but when all schemas fail the method only logs errors—it never throws. Callers have no way to detect validation failure.Silent error swallowing (lines 97-99): The try-catch logs errors but doesn't rethrow, so JSON serialization failures, socket errors, and validation errors are all swallowed silently.
Inconsistent return values: The no-schema path returns
{ isValid: true }(line 78), but the validation-success path (line 86-87) doesn't return anything.Apply this diff to fix the logic and error handling:
static ${methodName}(message, socket, schemas) { try { if (!schemas || schemas.length === 0) { socket.send(JSON.stringify(message)); return { isValid: true }; } const allValidationErrors = []; let isValid = false; for(const compiledSchema of schemas){ const validationResult = validateMessage(compiledSchema, message); if (validationResult.isValid) { isValid = true; socket.send(JSON.stringify(message)); - break; + return { isValid: true }; } else { if (validationResult.validationErrors) { allValidationErrors.push(...validationResult.validationErrors); } } - if (!isValid) { - console.error('Validation errors:', JSON.stringify(allValidationErrors, null, 2)); - } } + // If we reach here, validation failed against all schemas + console.error('Validation errors for ${methodName}:', JSON.stringify(allValidationErrors, null, 2)); + const error = new Error('Message validation failed for ${methodName} against all schemas.'); + error.validationErrors = allValidationErrors; + throw error; } catch (error) { - console.error('Error sending ${methodName} message:', error); + // Re-throw validation errors and other errors so callers can handle them + throw error; } }
♻️ Duplicate comments (1)
packages/components/src/components/SendOperations.js (1)
56-62: Return the validation result from the static method call.The instance method delegates to the static method but ignores its return value. The static method returns
{ isValid: true }when successful (line 78) and can throw validation errors. Either return the result to the caller or explicitly handle/document that this method has no return value and relies solely on exceptions for failure signaling.Apply this diff if you want to surface the validation result:
async ${methodName}(message){ if(!this.websocket){ throw new Error('WebSocket connection not established. Call connect() first.'); } await this.compileOperationSchemas(); const schemas = this.compiledSchemas['${methodName}']; - ${clientName}.${methodName}(message, this.websocket, schemas); + return ${clientName}.${methodName}(message, this.websocket, schemas); }
🧹 Nitpick comments (3)
apps/keeper/src/index.js (3)
6-14: Consider adding input validation for consistency.While
ajv.compilewill throw for invalid inputs, other functions in this file validate their parameters upfront. For consistency and clearer error messages, consider validating thatschemais an object.export function compileSchema(schema) { + if (!schema || typeof schema !== 'object') { + throw new Error('Invalid "schema" parameter: must be a non-null object.'); + } return ajv.compile(schema); }
77-84: Renamevalidatevariable toisValidfor clarity.The variable
validateholds a boolean validation result, not a validator function. Renaming it toisValidwould improve readability and prevent confusion.- const validate = compiledSchema(message); - if (validate) { + const isValid = compiledSchema(message); + if (isValid) { return { isValid: true }; }
65-65: Update JSDoc type annotation to reflect that null is allowed.The JSDoc declares
@param {object} message, but the code correctly allowsnullvalues (line 71 only throws forundefined). Update the type annotation to reflect this flexibility.- * @param {object} message - The message payload to validate. + * @param {*} message - The message payload to validate (null allowed if schema permits).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
package-lock.jsonis excluded by!**/package-lock.jsonpackages/components/test/components/__snapshots__/SendOperations.test.js.snapis excluded by!**/*.snappackages/templates/clients/websocket/test/integration-test/__snapshots__/integration.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
apps/keeper/package.json(1 hunks)apps/keeper/src/index.js(2 hunks)packages/components/src/components/SendOperations.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/keeper/package.json
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-12T14:23:48.919Z
Learnt from: derberg
Repo: asyncapi/generator PR: 1551
File: apps/generator/docs/generator-template.md:45-73
Timestamp: 2025-05-12T14:23:48.919Z
Learning: AsyncAPI 3.0.0 specification introduces significant structural changes from 2.x:
1. Operations become top-level elements with action property (send/receive) and references to channels
2. Channels use 'address' for the topic path instead of having nested publish/subscribe operations
3. Messages are defined under a 'messages' container in channels
4. The specification decouples operations, channels and messages to improve reusability
5. Servers can use a 'host' property
Applied to files:
packages/components/src/components/SendOperations.js
📚 Learning: 2025-05-12T14:23:48.919Z
Learnt from: derberg
Repo: asyncapi/generator PR: 1551
File: apps/generator/docs/generator-template.md:45-73
Timestamp: 2025-05-12T14:23:48.919Z
Learning: AsyncAPI 3.0.0 specification introduces significant structural changes from 2.x:
1. Operations become top-level elements with references to channels
2. Channels use 'address' for the topic path instead of having nested publish/subscribe
3. Messages are defined under a 'messages' container in channels
4. Servers can use a 'host' property
Applied to files:
packages/components/src/components/SendOperations.js
🧬 Code graph analysis (2)
packages/components/src/components/SendOperations.js (2)
packages/templates/clients/websocket/javascript/template/client.js.js (1)
clientName(10-10)packages/templates/clients/websocket/python/components/ClientClass.js (1)
clientName(13-13)
apps/keeper/src/index.js (2)
apps/generator/lib/templates/config/validator.js (1)
Ajv(2-2)apps/keeper/src/utils.js (3)
asyncapi(16-16)parseAsyncAPIDocumentFromFile(12-24)parseAsyncAPIDocumentFromFile(12-24)
⏰ 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 - ubuntu-latest
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - macos-13
🔇 Additional comments (3)
apps/keeper/src/index.js (3)
1-4: LGTM! Good Ajv configuration.The shared Ajv instance with
allErrors: trueensures comprehensive validation feedback, andstrict: falseprovides flexibility for various schema patterns.
16-31: LGTM! Clean implementation.The function properly validates the input array and delegates to
compileSchemafor individual schema compilation. The synchronous approach simplifies the API.
33-59: LGTM! Comprehensive error handling.The function properly validates inputs, handles missing operations with clear errors, and gracefully handles the no-messages case with a warning. The filter-map pattern for extracting payloads is clean and efficient.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/templates/clients/websocket/test/integration-test/__snapshots__/integration.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (1)
packages/templates/clients/websocket/javascript/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-12T14:57:35.024Z
Learnt from: Adi-204
Repo: asyncapi/generator PR: 1557
File: packages/templates/clients/websocket/javascript/components/SendOperation.js:21-21
Timestamp: 2025-05-12T14:57:35.024Z
Learning: In the AsyncAPI generator templates, particularly in the JavaScript WebSocket client, single quotes are used for string literals inside template-generated code (which is itself inside backticks) to preserve template placeholders like ${variable} as literal text in the generated code output.
Applied to files:
packages/templates/clients/websocket/javascript/package.json
📚 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/templates/clients/websocket/javascript/package.json
🪛 GitHub Actions: Test using test project
packages/templates/clients/websocket/javascript/package.json
[error] 1-1: npm JSON.parse error: Invalid JSON syntax in package.json (Expected ',' or '}' after property value). The log indicates a parsing error near a property value.
🔇 Additional comments (1)
packages/templates/clients/websocket/javascript/package.json (1)
13-17: Critical: @asyncapi/keeper dependency is not integrated into the template code.The dependency was added to
package.jsonbut verification reveals no actual integration:
- No keeper imports: Zero imports of
@asyncapi/keeperexist in any template files- Undefined function:
compileSchemasByOperationIdis called inCompileOperationSchemas.js:28but is not defined anywhere—this will cause a runtime error- No keeper API usage: Neither
compileSchemanorvalidateMessagefrom keeper are invoked anywhere in the template- No SendOperation validation: No validation logic exists to use keeper with SendOperation
Fixes required:
- Add keeper import to appropriate component(s)
- Define or import
compileSchemasByOperationId(or replace with keeper's actual API)- Implement message validation logic using keeper's APIs in SendOperation context
⛔ Skipped due to learnings
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.
| payload: | ||
| type: object | ||
| properties: | ||
| content: | ||
| type: string | ||
| description: The content to be echoed back | ||
| example: "test message" | ||
| messageId: | ||
| type: number | ||
| description: Unique identifier for the message | ||
| required: | ||
| - content |
There was a problem hiding this comment.
This changes is just for testing purpose and needs to revert before merging.
| this.compiledSchemas[operationId] = await compileSchemasByOperationId(asyncapiFilepath, operationId); | ||
| } | ||
| this.schemasCompiled = true; | ||
| console.log('Schemas initialized successfully for operations:', this.sendOperationsId.join(', ')); |
There was a problem hiding this comment.
This log is also for debugging can be removed before merging.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
apps/generator/package.json(1 hunks)
⏰ 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 - ubuntu-latest
- GitHub Check: Test NodeJS PR - macos-latest
- GitHub Check: Test NodeJS PR - windows-latest
apps/generator/package.json
Outdated
| "@asyncapi/generator-helpers": "*", | ||
| "@asyncapi/generator-hooks": "*", | ||
| "@asyncapi/generator-react-sdk": "*", | ||
| "@asyncapi/keeper": "*", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify package availability and consider tightening version constraint.
The new dependency uses a wildcard version ("*"), which permits any version including breaking changes. While this pattern is consistent with other internal @asyncapi/* packages in this file, it introduces supply chain risk and makes builds non-reproducible.
Consider the following verification steps:
- Confirm that
@asyncapi/keeperis properly published and accessible. - Verify the version constraint aligns with your project's dependency management standards.
- If possible, tighten the constraint to at least
"^<version>"(e.g.,"^1.0.0") to prevent accidental breaking changes during installation.
🏁 Script executed:
#!/bin/bash
# Description: Verify @asyncapi/keeper package availability and fetch latest version info
# Check if @asyncapi/keeper exists on npm and retrieve version details
curl -s "https://registry.npmjs.org/@asyncapi/keeper" | jq '{name: .name, "latest-version": .["dist-tags"].latest, versions: (.versions | keys | sort | .[-5:])}'Length of output: 94
Tighten version constraint to prevent unexpected updates from unstable package.
The new dependency uses a wildcard version ("*"), which is problematic for @asyncapi/keeper since it's a brand-new package (currently at 0.0.1 only). This allows any future version—including breaking changes in upcoming 0.1.0 or 1.0.0 releases—to install automatically without lockfile protection, breaking reproducible builds.
Change:
- "@asyncapi/keeper": "*",
+ "@asyncapi/keeper": "0.0.1",
Once @asyncapi/keeper reaches a stable 1.0.0 release, consider updating to "^1.0.0" to allow patch updates while preventing breaking changes.
🤖 Prompt for AI Agents
In apps/generator/package.json around line 59, the dependency "@asyncapi/keeper"
is using a wildcard version "*" which permits any future release (including
breaking changes) and undermines reproducible installs; change the version
string to the current specific release "0.0.1" to pin it, and later consider
switching to a caret range like "^1.0.0" once the package reaches stable 1.0.0
to allow safe updates.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
packages/templates/clients/websocket/javascript/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: derberg
Repo: asyncapi/generator PR: 1551
File: apps/generator/docs/generator-template.md:45-73
Timestamp: 2025-05-12T14:23:48.919Z
Learning: AsyncAPI 3.0.0 specification introduces significant structural changes from 2.x:
1. Operations become top-level elements with action property (send/receive) and references to channels
2. Channels use 'address' for the topic path instead of having nested publish/subscribe operations
3. Messages are defined under a 'messages' container in channels
4. The specification decouples operations, channels and messages to improve reusability
5. Servers can use a 'host' property
Learnt from: derberg
Repo: asyncapi/generator PR: 1551
File: apps/generator/docs/generator-template.md:45-73
Timestamp: 2025-05-12T14:23:48.919Z
Learning: AsyncAPI 3.0.0 specification introduces significant structural changes from 2.x:
1. Operations become top-level elements with references to channels
2. Channels use 'address' for the topic path instead of having nested publish/subscribe
3. Messages are defined under a 'messages' container in channels
4. Servers can use a 'host' property
📚 Learning: 2025-05-12T14:57:35.024Z
Learnt from: Adi-204
Repo: asyncapi/generator PR: 1557
File: packages/templates/clients/websocket/javascript/components/SendOperation.js:21-21
Timestamp: 2025-05-12T14:57:35.024Z
Learning: In the AsyncAPI generator templates, particularly in the JavaScript WebSocket client, single quotes are used for string literals inside template-generated code (which is itself inside backticks) to preserve template placeholders like ${variable} as literal text in the generated code output.
Applied to files:
packages/templates/clients/websocket/javascript/package.json
📚 Learning: 2025-05-12T17:29:03.886Z
Learnt from: derberg
Repo: asyncapi/generator PR: 1557
File: packages/templates/clients/websocket/javascript/components/SendOperation.js:7-7
Timestamp: 2025-05-12T17:29:03.886Z
Learning: Do not suggest adding key props to mapped React elements in the asyncapi/generator repository, even when linters like Biome flag them as missing.
Applied to files:
packages/templates/clients/websocket/javascript/package.json
📚 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/templates/clients/websocket/javascript/package.json
⏰ 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 18
- GitHub Check: Test generator as dependency with Node 20
- GitHub Check: Acceptance tests for generated templates
- GitHub Check: Test NodeJS PR - macos-latest
- GitHub Check: Test NodeJS PR - ubuntu-latest
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (1)
packages/templates/clients/websocket/javascript/package.json (1)
13-17: Keeper is not a template dependency; generated client would manage its own dependencies.The verification shows this PR only reorders dependencies alphabetically—no semantic changes. Keeper is generated by
DependencyProvideras output code in the client template, not as a template-generation requirement. Templatepackage.jsononly needs the tools to render the template (React SDK, helpers, components), not the dependencies of the generated output.Keeper was recently removed (commit 99d602b: "remove keeper from js"), and no keeper imports exist in the template code itself—only in the generated code strings that
DependencyProvideroutputs. The generated client would have its ownpackage.jsondeclaring keeper as a dependency, separate from this template's metadata.Likely an incorrect or invalid review comment.
| "eslint-plugin-react": "^7.34.1", | ||
| "eslint-plugin-sonarjs": "^0.5.0" | ||
| "eslint-plugin-sonarjs": "^0.5.0", | ||
| "jest-esm-transformer": "^1.0.0" |
There was a problem hiding this comment.
Remove unused jest-esm-transformer or integrate it into Jest configuration.
The jest-esm-transformer@^1.0.0 dependency (line 28) is added but not configured in the Jest setup. The transform config (line 37) still uses only babel-jest. Either remove the unused dependency or replace the babel-jest transformer with jest-esm-transformer. Babel already includes ESM handling via @babel/preset-env, so clarify the intent before keeping unused dependencies.
🤖 Prompt for AI Agents
In packages/templates/clients/websocket/javascript/package.json around line 28,
the dependency "jest-esm-transformer" is present but not referenced in the Jest
transform config (around line 37) so it is unused; either remove the
"jest-esm-transformer" entry from package.json (and run install/update lockfile)
or update the Jest config to replace or add the transformer (e.g., change the
transform entry to reference "jest-esm-transformer" instead of "babel-jest" and
remove "babel-jest" if no longer needed), and ensure any Babel vs ESM handling
is consistent (update other Jest settings or presets accordingly).
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/templates/clients/websocket/java/quarkus/jest.config.js (1)
1-13: Matches the shared Jest pattern; same optional DRY opportunityThis config correctly extends
baseConfig(__dirname)and applies the expectedmoduleFileExtensionsandbabel-jesttransform. It shares the same optional “centralize these common Jest options” refactor noted in the websocket dart Jest config.
🧹 Nitpick comments (5)
packages/templates/clients/websocket/dart/jest.config.js (1)
1-13: Config is fine; consider centralizing repeated Jest optionsThis setup (baseConfig +
moduleFileExtensions+babel-jesttransform) is correct and consistent. Since the exact same pattern appears in multiple template Jest configs, you could optionally pullmoduleFileExtensionsand the'^.+\\.jsx?$'→babel-jesttransform into a small shared helper or intojest.config.base.js(if all consumers need it) to avoid repetition.packages/templates/clients/websocket/test/javascript/jest.config.js (1)
1-13: Consistent Jest setup for websocket JS testsConfiguration is in line with the other template Jest configs and should work as expected. The only (optional) improvement is the same one already mentioned: consider centralizing the repeated
moduleFileExtensions/babel-jesttransform if you find more suites need the exact same settings.packages/templates/clients/websocket/test/integration-test/jest.config.js (1)
1-13: Integration-test Jest config is correct and alignedExtending
baseConfig(__dirname)and adding the standard JS/JSXbabel-jesttransform is appropriate here. This file also fits the same optional “factor out shared Jest options” refactor discussed for the other websocket template configs.packages/templates/clients/kafka/java/quarkus/jest.config.js (1)
1-13: Kafka Quarkus Jest config follows the shared patternThis is consistent with the other client template Jest configs and should work fine. As with the others, you could optionally extract the common
moduleFileExtensionsandbabel-jesttransform into shared config if you want to reduce repetition.packages/templates/clients/kafka/test/integration-test/jest.config.js (1)
1-13: Configuration looks correct, but consider consolidating duplicated settings.This Jest config correctly extends the base configuration. However, the
moduleFileExtensionsandtransformsettings are duplicated identically across multiple packages (components, websocket/javascript, and keeper). Consider moving these common settings to the base config or creating a shared preset.If consolidation is desired, the base config could accept an options parameter to enable JavaScript/JSX support:
// Example consolidation in jest.config.base.js module.exports = (dirname, options = {}) => { const config = { /* base config */ }; if (options.enableJavaScript) { config.moduleFileExtensions = ['js', 'json', 'jsx']; config.transform = { '^.+\\.jsx?$': 'babel-jest' }; } return config; };Then simplify per-package configs:
module.exports = baseConfig(__dirname, { enableJavaScript: true });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
apps/generator/jest.config.js(1 hunks)apps/generator/test/test-project/jest.config.js(1 hunks)apps/generator/test/test-project/package.json(0 hunks)apps/keeper/jest.config.js(1 hunks)apps/keeper/package.json(1 hunks)jest.config.base.js(1 hunks)packages/components/jest.config.js(1 hunks)packages/components/package.json(0 hunks)packages/helpers/jest.config.js(1 hunks)packages/helpers/package.json(0 hunks)packages/templates/clients/kafka/java/quarkus/jest.config.js(1 hunks)packages/templates/clients/kafka/java/quarkus/package.json(0 hunks)packages/templates/clients/kafka/test/integration-test/jest.config.js(1 hunks)packages/templates/clients/kafka/test/integration-test/package.json(0 hunks)packages/templates/clients/websocket/dart/jest.config.js(1 hunks)packages/templates/clients/websocket/dart/package.json(0 hunks)packages/templates/clients/websocket/java/quarkus/jest.config.js(1 hunks)packages/templates/clients/websocket/java/quarkus/package.json(0 hunks)packages/templates/clients/websocket/javascript/jest.config.js(1 hunks)packages/templates/clients/websocket/javascript/package.json(1 hunks)packages/templates/clients/websocket/python/jest.config.js(1 hunks)packages/templates/clients/websocket/python/package.json(0 hunks)packages/templates/clients/websocket/test/integration-test/jest.config.js(1 hunks)packages/templates/clients/websocket/test/integration-test/package.json(0 hunks)packages/templates/clients/websocket/test/javascript/jest.config.js(1 hunks)packages/templates/clients/websocket/test/javascript/package.json(0 hunks)
💤 Files with no reviewable changes (10)
- packages/helpers/package.json
- packages/components/package.json
- packages/templates/clients/websocket/java/quarkus/package.json
- packages/templates/clients/kafka/test/integration-test/package.json
- packages/templates/clients/websocket/test/integration-test/package.json
- packages/templates/clients/websocket/python/package.json
- packages/templates/clients/kafka/java/quarkus/package.json
- packages/templates/clients/websocket/test/javascript/package.json
- packages/templates/clients/websocket/dart/package.json
- apps/generator/test/test-project/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/templates/clients/websocket/javascript/package.json
- apps/keeper/package.json
🧰 Additional context used
🧬 Code graph analysis (14)
packages/components/jest.config.js (3)
apps/generator/jest.config.js (1)
baseConfig(2-2)apps/keeper/jest.config.js (1)
baseConfig(1-1)packages/helpers/jest.config.js (1)
baseConfig(1-1)
packages/templates/clients/websocket/javascript/jest.config.js (2)
packages/templates/clients/websocket/dart/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/python/jest.config.js (1)
baseConfig(1-1)
packages/templates/clients/websocket/java/quarkus/jest.config.js (8)
apps/generator/jest.config.js (1)
baseConfig(2-2)apps/generator/test/test-project/jest.config.js (1)
baseConfig(2-2)apps/keeper/jest.config.js (1)
baseConfig(1-1)packages/components/jest.config.js (1)
baseConfig(1-1)packages/helpers/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/dart/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/javascript/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/python/jest.config.js (1)
baseConfig(1-1)
packages/templates/clients/websocket/test/integration-test/jest.config.js (3)
packages/templates/clients/websocket/dart/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/javascript/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/test/javascript/jest.config.js (1)
baseConfig(1-1)
apps/generator/test/test-project/jest.config.js (2)
apps/generator/jest.config.js (2)
path(1-1)baseConfig(2-2)jest.config.base.js (1)
path(1-1)
packages/templates/clients/websocket/python/jest.config.js (4)
apps/generator/jest.config.js (1)
baseConfig(2-2)apps/generator/test/test-project/jest.config.js (1)
baseConfig(2-2)packages/templates/clients/websocket/dart/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/javascript/jest.config.js (1)
baseConfig(1-1)
packages/templates/clients/websocket/test/javascript/jest.config.js (2)
packages/templates/clients/websocket/javascript/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/test/integration-test/jest.config.js (1)
baseConfig(1-1)
packages/templates/clients/kafka/java/quarkus/jest.config.js (4)
packages/templates/clients/kafka/test/integration-test/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/java/quarkus/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/test/integration-test/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/test/javascript/jest.config.js (1)
baseConfig(1-1)
packages/helpers/jest.config.js (12)
apps/generator/jest.config.js (1)
baseConfig(2-2)apps/generator/test/test-project/jest.config.js (1)
baseConfig(2-2)apps/keeper/jest.config.js (1)
baseConfig(1-1)packages/components/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/kafka/java/quarkus/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/kafka/test/integration-test/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/dart/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/java/quarkus/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/javascript/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/python/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/test/integration-test/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/test/javascript/jest.config.js (1)
baseConfig(1-1)
jest.config.base.js (3)
apps/generator/jest.config.js (1)
path(1-1)apps/generator/test/test-project/jest.config.js (1)
path(1-1)apps/generator/lib/parser.js (1)
options(24-24)
packages/templates/clients/websocket/dart/jest.config.js (5)
packages/templates/clients/websocket/java/quarkus/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/javascript/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/python/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/test/integration-test/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/test/javascript/jest.config.js (1)
baseConfig(1-1)
apps/keeper/jest.config.js (6)
apps/generator/jest.config.js (1)
baseConfig(2-2)apps/generator/test/test-project/jest.config.js (1)
baseConfig(2-2)packages/components/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/kafka/java/quarkus/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/javascript/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/test/javascript/jest.config.js (1)
baseConfig(1-1)
apps/generator/jest.config.js (1)
jest.config.base.js (1)
path(1-1)
packages/templates/clients/kafka/test/integration-test/jest.config.js (6)
apps/generator/jest.config.js (1)
baseConfig(2-2)apps/generator/test/test-project/jest.config.js (1)
baseConfig(2-2)packages/templates/clients/kafka/java/quarkus/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/java/quarkus/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/test/integration-test/jest.config.js (1)
baseConfig(1-1)packages/templates/clients/websocket/test/javascript/jest.config.js (1)
baseConfig(1-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). (6)
- GitHub Check: Acceptance tests for generated templates
- GitHub Check: Test generator as dependency with Node 18
- GitHub Check: Test generator as dependency with Node 20
- GitHub Check: Test NodeJS PR - macos-latest
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - ubuntu-latest
🔇 Additional comments (8)
packages/templates/clients/websocket/python/jest.config.js (1)
1-13: ****The file location is correct. The
pythondirectory name refers to the target language being generated (Python client), not the language of the template files. The directory contains JavaScript template files and components that generate Python output—this is a standard pattern for code generators. The jest.config.js correctly configures testing for the JavaScript template components in this directory.The multiple
.jsfiles inpackages/templates/clients/websocket/python/components/,template/, andtest/confirm the templates are written in JavaScript, and the jest.config.js appropriately tests them.Likely an incorrect or invalid review comment.
apps/generator/test/test-project/jest.config.js (1)
1-11: Jest config and path aliases look correctThe base config usage and the relative paths for the local
@asyncapi/*packages line up with the directory depth of this test project; I don’t see any issues here.jest.config.base.js (1)
1-24: Base Jest config and nimma mappings look soundThe relative-path calculation back to the repo root and the resulting nimma
moduleNameMapperentries are correct, and merging caller-specific mappings after the defaults gives a sensible override mechanism. I don’t see any functional problems with this base config.packages/helpers/jest.config.js (1)
1-5: Helpers Jest config correctly delegates to the base configUsing
baseConfig(__dirname)without extra overrides keeps this package’s tests aligned with the shared defaults; looks good.apps/generator/jest.config.js (1)
2-9: LGTM!The refactoring to extend the base Jest config while preserving the package-specific moduleNameMapper is clean and maintains the necessary local path resolution for nunjucks-filters.
packages/components/jest.config.js (1)
1-13: LGTM!The Jest configuration correctly extends the base config with appropriate JavaScript/JSX support.
packages/templates/clients/websocket/javascript/jest.config.js (1)
1-13: LGTM!The configuration correctly extends the base config with appropriate settings for JavaScript template testing.
apps/keeper/jest.config.js (1)
1-13: LGTM!The Keeper Jest configuration correctly extends the base config and aligns with the broader refactoring to centralize Jest setup across the monorepo.
|
all looks good, you can remove changes that were there just for testing the only problem I see that when I run locally integration tests, snapshot fail and update removes your functionality. Did I miss something? 
|
|
@derberg that is weird because in PR integration test snapshot we can see my changes are added and also CI would have failed but all are passing. Probably it is your local machine issue 😉 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpackages/templates/clients/websocket/test/integration-test/__snapshots__/integration.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (1)
packages/templates/clients/kafka/java/quarkus/.ageneratorrc(1 hunks)
⏰ 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 18
- GitHub Check: Test generator as dependency with Node 20
- GitHub Check: Acceptance tests for generated templates
- GitHub Check: Test NodeJS PR - macos-latest
- GitHub Check: Test NodeJS PR - ubuntu-latest
- GitHub Check: Test NodeJS PR - windows-latest
|
one last thing, why |
Adi-204
changed the title
@asyncapi/keeper for message validation in JS websocket client@asyncapi/keeper and use for message validation in JS websocket client
|
|
/rtm |
…S` websocket client (asyncapi#1747) Co-authored-by: Adi-204 <[email protected]> Co-authored-by: Adi Boghawala <[email protected]> Co-authored-by: Chan <[email protected]> Co-authored-by: Lukasz Gornicki <[email protected]>
3 participants
Description
Add message validation before sending messages using library ajv. Locally I change client_postman fixture by changing schema for sendEchoMessage operation it is just for testing purpose we will revert fixture changes.
Related issue(s)
Fixes #1665
In order to start working on #1548 I need a working validator.
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.