-
-
Notifications
You must be signed in to change notification settings - Fork 336
feat!: remove nunjucks support and ag CLI
#1756
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 112a5ab 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 |
WalkthroughRemoves Nunjucks renderer and nunjucks-filters package, makes React the sole renderer, deletes filter-registration and watcher/CLI watch code, updates generator core and template manifests, and removes Nunjucks-related tests and documentation across the repo. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 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 |
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.
|
|
🚀 Docs preview deployed |
|
🚀 Docs preview deployed |
|
🚀 Docs preview deployed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Development.md (1)
61-61: Clarify path references for template location.Line 61 states the template is located at
apps/generator/test/templates, but the instructions at lines 68 and 73 reference it at./test/test-templates/react-template. Ensure the path references are consistent so developers can accurately follow the manual testing steps without confusion.Update line 61 to align with the actual path used in the instructions:
-To test template features manually we have `react-template` in `apps/generator/test/templates`, you can use this template to manually test your changes like this: +To test template features manually we have `react-template` in `./test/test-templates`, you can use this template to manually test your changes like this:Alternatively, if
./test/test-templates/react-templateis the incorrect path, update lines 68 and 73 accordingly.Also applies to: 68-68, 73-73
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
apps/generator/test/__snapshots__/integration.test.js.snapis excluded by!**/*.snapapps/generator/test/test-templates/nunjucks-template/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (31)
Development.md(1 hunks)README.md(0 hunks)apps/generator/docs/typescript-support.md(1 hunks)apps/generator/jest.config.js(0 hunks)apps/generator/lib/filtersRegistry.js(0 hunks)apps/generator/lib/generator.js(4 hunks)apps/generator/lib/renderer/nunjucks.js(0 hunks)apps/generator/lib/templates/config/validator.js(1 hunks)apps/generator/lib/utils.js(0 hunks)apps/generator/package.json(0 hunks)apps/generator/test/generator.test.js(0 hunks)apps/generator/test/integration.test.js(0 hunks)apps/generator/test/templateConfigValidator.test.js(1 hunks)apps/generator/test/test-project/package.json(0 hunks)apps/generator/test/test-templates/nunjucks-template/package.json(0 hunks)apps/generator/test/test-templates/nunjucks-template/template/test-file.md(0 hunks)apps/generator/test/test-templates/react-template/.ageneratorrc(0 hunks)apps/generator/test/utils.test.js(0 hunks)apps/nunjucks-filters/docs/api.md(0 hunks)apps/nunjucks-filters/package.json(0 hunks)apps/nunjucks-filters/src/customFilters.js(0 hunks)apps/nunjucks-filters/src/customFilters.test.js(0 hunks)apps/nunjucks-filters/src/index.js(0 hunks)apps/nunjucks-filters/src/lodashFilters.js(0 hunks)apps/nunjucks-filters/src/lodashFilters.test.js(0 hunks)package.json(0 hunks)packages/templates/clients/kafka/java/quarkus/.ageneratorrc(0 hunks)packages/templates/clients/websocket/dart/.ageneratorrc(0 hunks)packages/templates/clients/websocket/java/quarkus/.ageneratorrc(0 hunks)packages/templates/clients/websocket/javascript/.ageneratorrc(0 hunks)packages/templates/clients/websocket/python/.ageneratorrc(0 hunks)
💤 Files with no reviewable changes (26)
- apps/generator/test/integration.test.js
- apps/nunjucks-filters/docs/api.md
- apps/generator/test/generator.test.js
- apps/generator/lib/filtersRegistry.js
- apps/generator/lib/utils.js
- package.json
- packages/templates/clients/kafka/java/quarkus/.ageneratorrc
- packages/templates/clients/websocket/java/quarkus/.ageneratorrc
- apps/nunjucks-filters/src/lodashFilters.test.js
- apps/nunjucks-filters/src/customFilters.test.js
- apps/generator/package.json
- apps/nunjucks-filters/src/lodashFilters.js
- packages/templates/clients/websocket/javascript/.ageneratorrc
- apps/nunjucks-filters/src/index.js
- apps/generator/lib/renderer/nunjucks.js
- apps/generator/test/test-project/package.json
- packages/templates/clients/websocket/dart/.ageneratorrc
- apps/nunjucks-filters/src/customFilters.js
- apps/generator/test/utils.test.js
- apps/generator/test/test-templates/nunjucks-template/template/test-file.md
- apps/nunjucks-filters/package.json
- packages/templates/clients/websocket/python/.ageneratorrc
- README.md
- apps/generator/jest.config.js
- apps/generator/test/test-templates/nunjucks-template/package.json
- apps/generator/test/test-templates/react-template/.ageneratorrc
🧰 Additional context used
🧬 Code graph analysis (2)
apps/generator/lib/templates/config/validator.js (1)
apps/generator/lib/templates/config/loader.js (1)
templateConfig(17-17)
apps/generator/lib/generator.js (1)
apps/react-sdk/src/utils/isJsFile.ts (1)
isJsFile(9-12)
🪛 markdownlint-cli2 (0.18.1)
apps/generator/docs/typescript-support.md
6-6: Link fragments should be valid
(MD051, link-fragments)
8-8: Spaces inside code span elements
(MD038, no-space-in-code)
⏰ 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). (1)
- GitHub Check: preview
🔇 Additional comments (8)
Development.md (1)
61-61: Properly reflects the Nunjucks removal and React consolidation.The change to reference only the
react-templatealigns well with the PR's objective of removing Nunjucks and making React the sole rendering engine. The wording is clear and accurate.apps/generator/docs/typescript-support.md (1)
6-11: Documentation correctly updated.The removal of filters references aligns with the nunjucks deprecation, and the TypeScript support documentation now accurately reflects hooks-only support.
apps/generator/lib/templates/config/validator.js (1)
133-144: LGTM! Validator correctly updated for React-only rendering.The changes properly remove nunjucks from supported renderers and update the error message accordingly. The logic correctly allows
undefined(defaulting to react) and explicit'react'values.apps/generator/test/templateConfigValidator.test.js (1)
36-42: LGTM! Test expectations align with validator changes.The error message expectation correctly reflects that only 'react' is now supported. The test properly validates the validator's behavior for unsupported renderers.
apps/generator/lib/generator.js (4)
402-406: LGTM! Simplified configuration logic.The conditional configuration correctly handles the
compileflag: whentrue, it compiles the React template; whenfalse, it uses the pre-transpiled version fromTRANSPILED_TEMPLATE_LOCATION. This aligns with the removal of nunjucks-specific configuration.
866-872: LGTM! Rendering path simplified to React-only.The rendering workflow now exclusively uses React-based rendering, which is correct given the nunjucks removal. The function correctly delegates to
saveRenderedReactContentfor all rendered output.
982-987: LGTM! Non-renderable file detection simplified.The simplified logic correctly identifies non-renderable files for React templates by checking if the file is not a JavaScript/TypeScript file. This is appropriate since React templates require JS/TS files for rendering.
971-973: Verify that external templates receive proper migration guidance for breaking change.The change to
renderFileexclusively usingrenderReactis confirmed as a breaking change. All baked-in templates in this codebase appear to be React-compatible (no Nunjucks rendering logic remains in the codebase). However, any external or community templates that specify"renderer": "nunjucks"in their configuration will break. Ensure:
- A clear breaking change notice is documented
- Migration guidance is provided for external template maintainers
- Error messages guide users if they use a Nunjucks template
| --- | ||
|
|
||
| The AsyncAPI generator has TypeScript support for [hooks](#hooks) and Nunjucks's [filters](#filters). Assumptions: | ||
| The AsyncAPI generator has TypeScript support for [hooks](#hooks). Assumptions: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify the link fragment for hooks.
The markdown linter flags that the link fragment #hooks may be invalid. Please verify this anchor exists in the target document to ensure navigation works correctly.
Based on static analysis hints.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
6-6: Link fragments should be valid
(MD051, link-fragments)
🤖 Prompt for AI Agents
In apps/generator/docs/typescript-support.md around line 6, the link fragment
`#hooks` may be invalid; verify the target document contains a heading that
generates the `hooks` anchor (case-insensitive, spaces/punctuation normalized)
and if it does not, either update the link to the correct fragment (e.g.,
`#hook-usage`, `#lifecycle-hooks`, or the exact heading slug) or add the
corresponding "Hooks" heading to the document so the anchor resolves; ensure the
fragment matches the actual generated Markdown heading slug.
…generator into nunjucksfinallygoesaway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/generator/docs/file-templates.md (1)
8-8: Optional: Consider minor wording refinement for conciseness.The phrase "large number of files with varying content" is clear but slightly wordy. For brevity, you could simplify to "many files" or "numerous files."
📜 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 (12)
apps/generator/docs/api.md(1 hunks)apps/generator/docs/asyncapi-document.md(1 hunks)apps/generator/docs/configuration-file.md(0 hunks)apps/generator/docs/file-templates.md(1 hunks)apps/generator/docs/generator-template-java.md(0 hunks)apps/generator/docs/generator-template.md(1 hunks)apps/generator/docs/index.md(1 hunks)apps/generator/docs/migration-nunjucks-react.md(0 hunks)apps/generator/docs/nunjucks-render-engine.md(0 hunks)apps/generator/docs/react-render-engine.md(1 hunks)apps/generator/docs/template-development.md(2 hunks)apps/generator/docs/template.md(1 hunks)
💤 Files with no reviewable changes (4)
- apps/generator/docs/migration-nunjucks-react.md
- apps/generator/docs/configuration-file.md
- apps/generator/docs/generator-template-java.md
- apps/generator/docs/nunjucks-render-engine.md
✅ Files skipped from review due to trivial changes (2)
- apps/generator/docs/api.md
- apps/generator/docs/react-render-engine.md
🧰 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:
apps/generator/docs/template-development.mdapps/generator/docs/template.mdapps/generator/docs/file-templates.md
📚 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:
apps/generator/docs/template-development.md
🪛 LanguageTool
apps/generator/docs/file-templates.md
[style] ~8-~8: To reduce wordiness, try specifying a number or using “many” or “numerous” instead.
Context: ... templates or when you need to generate a large number of files with varying content. ### Exampl...
(LARGE_NUMBER_OF)
⏰ 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)
- GitHub Check: Acceptance tests for generated templates
- GitHub Check: Test generator as dependency with Node 20
- GitHub Check: Test generator as dependency with Node 18
- GitHub Check: preview
🔇 Additional comments (8)
apps/generator/docs/asyncapi-document.md (1)
24-24: Approved: Clear and accurate restriction to React render engine.The change correctly specifies that Template Context is accessible to the React render engine. The reference link
[react](react-render-engine)provides proper documentation linkage.apps/generator/docs/file-templates.md (1)
8-8: Approved: React multi-file capability clearly documented.The updated description accurately explains the React render engine's support for returning arrays of
Filecomponents. The examples remain valid and demonstrate practical use cases.apps/generator/docs/index.md (1)
27-28: Approved: Accurate single-engine (React) architecture description.The updated steps 6 and 7 correctly specify the React render engine and its role in template processing. The transition from multi-engine to single-engine messaging is clear and aligns with PR objectives.
apps/generator/docs/template.md (1)
48-49: Approved: Clear and accurate React render engine specification.The updated steps 5 and 6 explicitly identify the React render engine and accurately describe the template processing flow. This change resolves any ambiguity from the original dual-engine documentation.
apps/generator/docs/template-development.md (2)
6-6: Approved: Accurate reference to React render engine in template guidance.The updated note correctly specifies the React render engine when advising against manual templating of models. This aligns with the removal of multi-engine support.
43-43: Clarify intent: Is the empty"generator": {}object intentional?The example shows an empty
generatorobject, which is valid. However, verify this is intentionally minimal (e.g., for simplicity in the example) versus incomplete. If developers typically include configuration options likeapiVersionorsupportedProtocols, the example may need expansion for completeness.apps/generator/docs/generator-template.md (2)
114-118: Approved: Accurate and complete package.json generator configuration example.The example correctly omits the
rendererfield and includes the necessaryapiVersion,generator, andsupportedProtocolsfields. This aligns with the PR's removal of Nunjucks and the renderer option, establishing React as the implicit default.
328-332: Approved: Consistent generator configuration across all examples.The later example in the scripts section maintains the same configuration pattern without the deprecated
rendererfield, ensuring consistency throughout the tutorial.
|
Tests are green but of what is left:
And I did not even touch CLI |
|
/bounty |
|
@asyncapi/bounty_team |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
deprecated cli |
ag CLI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/remove-deprecated-nunjucks-cli.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
.changeset/remove-deprecated-nunjucks-cli.md
6-6: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
7-7: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
8-8: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
⏰ 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 20
- GitHub Check: Test generator as dependency with Node 18
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - ubuntu-latest
- GitHub Check: preview
🔇 Additional comments (1)
.changeset/remove-deprecated-nunjucks-cli.md (1)
11-12: Verify version tag in migration guide links.The migration guide links reference a specific version tag (
@asyncapi/generator@2.8.4), which seems misaligned for a major version release. For a breaking change that introduces a new major version, these links should likely reference a more general path (e.g., the main branch or the new version tag) rather than a prior release version.Please confirm whether this version reference is intentional or should be updated.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
README.md (1)
21-25: TOC still links to removed “Filters” sectionThe table of contents includes
- [Filters](#filters), but there is no longer a## Filterssection in the README. This leaves a broken anchor after the filters/Nunjucks removal.Apply a small diff to drop the stale TOC entry:
- - [List of official generator templates](#list-of-official-generator-templates) -- [Filters](#filters) -- [Hooks](#hooks) + - [List of official generator templates](#list-of-official-generator-templates) + - [Hooks](#hooks)
🧹 Nitpick comments (1)
README.md (1)
64-69: Hooks section text is consistent with hook behaviorThe Hooks section now gives a concise, accurate description of how hooks are invoked during generation and points to the dedicated hooks docs. This matches the current generator behavior and React‑only rendering model.
If you want slightly tighter copy, consider reducing repetition and tightening phrasing:
-[Hooks](https://www.asyncapi.com/docs/tools/generator/hooks) are functions called by the generator at specific moments in the generation process. Hooks can be anonymous functions, but you can also assign them function names. These hooks can have arguments provided to them, or they may be expected to return a value. +[Hooks](https://www.asyncapi.com/docs/tools/generator/hooks) are functions the generator calls at specific moments in the generation process. They can be anonymous or named and may accept arguments or return values, depending on the hook type.
📜 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 (7)
.changeset/remove-deprecated-nunjucks-cli.md(1 hunks)Development.md(2 hunks)README.md(2 hunks)apps/generator/docs/typescript-support.md(1 hunks)apps/generator/lib/watcher.js(0 hunks)apps/generator/package.json(0 hunks)apps/generator/test/cli.js(1 hunks)
💤 Files with no reviewable changes (2)
- apps/generator/lib/watcher.js
- apps/generator/package.json
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/generator/docs/typescript-support.md
- Development.md
- .changeset/remove-deprecated-nunjucks-cli.md
🧰 Additional context used
🧠 Learnings (1)
📚 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:
README.md
🪛 LanguageTool
README.md
[style] ~66-~66: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ific moments in the generation process. Hooks can be anonymous functions, but you can...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ 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). (7)
- 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-latest
- GitHub Check: preview
🔇 Additional comments (3)
apps/generator/test/cli.js (2)
3-9: Deprecation header is clear and aligned with the migration storyThe new header communicates that this CLI is deprecated, test-only, and points users to AsyncAPI CLI, which matches the broader deprecation/removal goals. No changes needed here.
16-18: Updated imports correctly target production modulesSwitching the requires to
../package.json,../lib/generator, and../lib/utilsmakes this test CLI use the same modules as production. This looks correct given the file’s location underapps/generator/test/.Please just double‑check locally that any tests or docs that reference this script still work with the new import paths and that there are no surprises from picking up the real
lib/generatorimplementation.README.md (1)
7-7: Hooks package description remains accurate post‑Nunjucks removalThe updated one‑liner for
apps/hooksstill accurately explains the purpose of hooks in the generation lifecycle and fits the new React‑only architecture. No change required.
|



TODO:
Resolves: #1720
Breaking Changes
agCLI is no longer available. Long time ago it was already announced that it will be removed and people should move to AsyncAPI CLI. Old CLI code without watcher is still there but moved to tests as purely local-dev solutionMigration Guides
agCLI to AsyncAPI CLISummary by CodeRabbit
Release Notes
Removed Features
Breaking Changes
✏️ Tip: You can customize this high-level summary in your review settings.