feat: add the .ageneratorrc file for setting the generator Config. #1485
Conversation
ItshMoh
requested review from
derberg,
magicmatatjahu,
jonaslagoni and
asyncapi-bot-eve
as code owners
|
WalkthroughThe pull request updates documentation and configuration handling in the generator module. In Changes
Sequence Diagram(s)sequenceDiagram
participant G as Generator
participant FS as File System
participant YP as YAML Parser
G->>FS: Check if .ageneratorrc exists
alt .ageneratorrc found
FS-->>G: Return .ageneratorrc content
G->>YP: Parse YAML content
YP-->>G: Parsed configuration
G-->>G: Set templateConfig
else .ageneratorrc missing
G->>FS: Check for package.json
FS-->>G: Return package.json content
G-->>G: Extract "generator" config or use {}
end
Note right of G: Log error if any exception occurs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/generator/lib/generator.js (1)
971-992: Good implementation of the new.ageneratorrcconfiguration.The method now properly prioritizes
.ageneratorrcoverpackage.json, with appropriate fallback behavior and error handling. This change effectively implements the feature to allow users to define configurations outside of package.json.A few suggestions for improvement:
- Consider adding a constant for the
.ageneratorrcfilename at the top of the file with the other constants (similar toCONFIG_FILENAME)- Consider using the existing logging system (
logobject) instead ofconsole.errorfor consistency with the rest of the codebase- console.error('Error loading template config:', e); + log.error('Error loading template config:', e);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
apps/generator/lib/filtersRegistry.js(1 hunks)apps/generator/lib/generator.js(2 hunks)package.json(1 hunks)packages/templates/clients/websocket/javascript/.ageneratorrc(1 hunks)packages/templates/clients/websocket/javascript/package.json(0 hunks)packages/templates/clients/websocket/python/.ageneratorrc(1 hunks)packages/templates/clients/websocket/python/package.json(0 hunks)
💤 Files with no reviewable changes (2)
- packages/templates/clients/websocket/javascript/package.json
- packages/templates/clients/websocket/python/package.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test generator as dependency with Node 20
- GitHub Check: Test generator as dependency with Node 18
🔇 Additional comments (6)
package.json (1)
47-49: Addition of js-yaml dependency looks good.The added dependency
js-yaml(version ^4.1.0) is necessary to support parsing the newly introduced.ageneratorrcYAML configuration files. This aligns perfectly with the PR objectives to allow users to define generator configurations in a.ageneratorrcfile as an alternative to using thepackage.jsonfile.apps/generator/lib/filtersRegistry.js (1)
19-19: Documentation update correctly reflects new configuration options.The comment has been updated to include both
package.jsonand.ageneratorrcas places where Nunjucks filters don't need to be listed. This documentation change ensures clarity for developers working with the system and is consistent with the new feature implementation.packages/templates/clients/websocket/javascript/.ageneratorrc (1)
1-11: Well-structured configuration file for JavaScript WebSocket client.This newly added
.ageneratorrcfile correctly implements the generator configuration for the JavaScript WebSocket client with appropriate renderer, API version, and generator version constraints. The parameters section is well-defined with clear descriptions and proper required/default value handling.According to the AI summary, this configuration was previously embedded in the
package.jsonfile and has now been properly extracted to this dedicated configuration file, which aligns with the PR objectives.packages/templates/clients/websocket/python/.ageneratorrc (1)
1-11: Well-structured configuration file for Python WebSocket client.This newly added
.ageneratorrcfile correctly implements the generator configuration for the Python WebSocket client. The configuration properly sets the renderer to React, API version to v3, and defines appropriate generator version constraints. The parameters section correctly defines the required server parameter and optional clientFileName parameter with an appropriate Python-specific default value (client.py).According to the AI summary, this configuration was previously embedded in the
package.jsonfile and has now been properly extracted to this dedicated configuration file, which aligns with the PR objectives.apps/generator/lib/generator.js (2)
138-138: Error message updated to reference the new configuration file.The error message now correctly references
.ageneratorrcinstead ofpackage.json, aligning with the new configuration approach.
976-978: Verify js-yaml dependency has been added.The code now depends on the
js-yamllibrary to parse the YAML configuration file. Let's verify this dependency has been properly added to the project.#!/bin/bash # Check if js-yaml is listed in package.json dependencies grep -A 10 '"dependencies"' package.json | grep 'js-yaml' # Check if js-yaml is actually installed in node_modules find node_modules -name "js-yaml" -type d | head -1
|
hey @derberg here I have allowed the both |
|
/u |
|
| const yamlConfig = require('js-yaml').load(yaml); | ||
| this.templateConfig = yamlConfig || {}; | ||
| } catch (accessError) { | ||
| // File doesn't exist, fallback to package.json |
There was a problem hiding this comment.
IMHO, it would be better to explicitly check for the presence of the file and doing conditional code, instead of relying on throwing errors and catching them.
Reasons:
1- You have enclosed multiple operations in the try block, not only the access function. If one of the other operations produce an error such as readFile (unrelated to file not existing) or the require().load(), it could be wrongly interpreted as file not existing. This is also for the second try/catch below.
2- There are multiple nested try/catch, and I believe this reduces readability and makes it harder to maintain. It would be much easier if it was a linear flow with conditionals instead.
Proposal: replace .access with a method that checks for the presence of file and returns a boolean.
There was a problem hiding this comment.
I looked up the methods and it seems that the promisified exist method is deprecated.
I guess if we don't want to use existsSync, then this is a good option: https://stackoverflow.com/a/65446343/16761632
There was a problem hiding this comment.
On another note, while looking through the docs, I found out an explicit note to not use .access to check if the file exists before reading, instead reading directly and handling the error if it doesn't exist: https://nodejs.org/api/fs.html#fsaccesspath-mode-callback
Description
In this PR i have added the feature for .ageneratorrc file which can be defined in the template directory and here we can write the generator config. By this file we can replace the generator key from the package.json. I have changed the
loadTemplateConfigfunction to allow the .ageneratorrc file. It is just optional to add .ageneratorrc file you can also do the config settings in package.json also.Related issue(s)
#1449
Summary by CodeRabbit
New Features
Bug Fixes
Chores