feat(cli): replace banner rendering with echo-banner#149
feat(cli): replace banner rendering with echo-banner#149teneplaysofficial wants to merge 4 commits into
Conversation
🦋 Changeset detectedLatest commit: c7a7fb0 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 |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 14 minutes and 37 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the figlet library with echo-banner for CLI banner rendering and updates the project's ESLint configuration. Feedback suggests that disabling reportUnusedDisableDirectives globally may hinder maintainability by allowing stale comments to persist. Furthermore, it is recommended to provide proper TypeScript definitions for the echo-banner package to avoid the need for manual ESLint suppressions when calling its print function.
| { | ||
| linterOptions: { | ||
| reportUnusedDisableDirectives: false, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Disabling reportUnusedDisableDirectives globally can lead to stale eslint-disable comments in the codebase, which harms maintainability. It's better to keep this enabled (the default is usually 'warn') to get feedback on unnecessary disable directives.
If a disable directive is incorrectly reported as unused due to different environments, consider using a more specific configuration or adding a description to the eslint-disable comment itself.
| } | ||
|
|
||
| printBanner(); | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-call |
There was a problem hiding this comment.
Using eslint-disable-next-line for @typescript-eslint/no-unsafe-call should be a last resort. Since echo-banner appears to be a package within this monorepo, the best solution would be to add proper types to the print function in the echo-banner package itself. This would provide type safety and improve developer experience across the project.
If fixing the types is not immediately possible, it's good practice to add a descriptive comment to the disable directive explaining why it's necessary, for example:
// eslint-disable-next-line @typescript-eslint/no-unsafe-call -- Follow-up task: Add types to echo-bannerThis provides context for future developers.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
eslint.config.ts (1)
16-20: GlobalreportUnusedDisableDirectives: falseis overly broad.This setting disables unused directive warnings across the entire codebase, but it appears to have been added to support a single
eslint-disable-next-lineinpackages/juk-cli/src/index.ts. Consider either:
- Removing this setting entirely—the directive in
index.tsis actively used and won't trigger an "unused" warning.- If you need to suppress specific directives, scope this to the relevant files only.
The underlying issue is that
echo-bannerlacks TypeScript type declarations. Addressing that (seeindex.tscomment) would remove the need for any suppression.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eslint.config.ts` around lines 16 - 20, Remove the global linterOptions.reportUnusedDisableDirectives: false from eslint.config.ts and instead either (a) delete the lone eslint-disable-next-line in packages/juk-cli/src/index.ts if it's no longer needed, or (b) restrict suppression to only that file by adding a per-file overrides entry in eslint.config.ts that sets reportUnusedDisableDirectives for the pattern matching packages/juk-cli/src/index.ts; additionally add or create TypeScript type declarations for the echo-banner module (or a local .d.ts with declare module "echo-banner") so the disable directive becomes unnecessary.packages/juk-cli/src/index.ts (1)
35-38: ESLint suppression can be removed once types are added.The
@typescript-eslint/no-unsafe-callsuppression is a symptom of missing type declarations. Once you add the type declaration file (see previous comment), this suppression should be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/juk-cli/src/index.ts` around lines 35 - 38, Remove the eslint suppression and add proper TypeScript types so the call to print({ pkg }) is type-safe: declare or import the correct type for the pkg variable (e.g., PackageJson or a custom IPackage interface) and ensure the print function has an appropriate typed signature (e.g., print(opts: { pkg: IPackage }): Promise<void>), then remove the line "// eslint-disable-next-line `@typescript-eslint/no-unsafe-call`" so the linter can validate the call to print and pkg.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pnpm-workspace.yaml`:
- Line 6: The workspace lists an unpublished/experimental package "echo-banner"
in pnpm-workspace.yaml which will break dependency resolution; remove or replace
the echo-banner entry (or pin it to a valid published version) and instead add a
stable published alternative or a local workspace reference that actually
exists; ensure pnpm install succeeds after updating pnpm-workspace.yaml and
verify the package is published on npm (or convert to a local package folder and
reference it via its workspace name) before reintroducing echo-banner.
---
Nitpick comments:
In `@eslint.config.ts`:
- Around line 16-20: Remove the global
linterOptions.reportUnusedDisableDirectives: false from eslint.config.ts and
instead either (a) delete the lone eslint-disable-next-line in
packages/juk-cli/src/index.ts if it's no longer needed, or (b) restrict
suppression to only that file by adding a per-file overrides entry in
eslint.config.ts that sets reportUnusedDisableDirectives for the pattern
matching packages/juk-cli/src/index.ts; additionally add or create TypeScript
type declarations for the echo-banner module (or a local .d.ts with declare
module "echo-banner") so the disable directive becomes unnecessary.
In `@packages/juk-cli/src/index.ts`:
- Around line 35-38: Remove the eslint suppression and add proper TypeScript
types so the call to print({ pkg }) is type-safe: declare or import the correct
type for the pkg variable (e.g., PackageJson or a custom IPackage interface) and
ensure the print function has an appropriate typed signature (e.g., print(opts:
{ pkg: IPackage }): Promise<void>), then remove the line "//
eslint-disable-next-line `@typescript-eslint/no-unsafe-call`" so the linter can
validate the call to print and pkg.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ada0fc2-f09b-46c4-8174-4d7d4052c2fc
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.changeset/common-lands-walk.mdeslint.config.tspackage.jsonpackages/juk-cli/package.jsonpackages/juk-cli/src/index.tspackages/juk-cli/src/printBanner.tspnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- packages/juk-cli/src/printBanner.ts
| catalog: | ||
| archiver: ^7.0.1 | ||
| figlet: ^1.11.0 | ||
| echo-banner: ^0.3.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
echo-banner npm package documentation and stability
💡 Result:
The "echo-banner" npm package is a new utility for creating clean metadata banners in builds and beautiful ASCII titles for CLI applications. Official documentation and source code are available on GitHub at https://github.com/teneplaysofficial/echo-banner. Installation is via npm install echo-banner. No npm registry page was found in searches, suggesting it may not yet be published to npm or is very new (repo published 2026-03-19). The same author (teneplaysofficial/tenedev) has a recent published package @tenedev/react-slideshow (Apr 2025). Stability: Low confidence due to recent creation (9 days ago as of 2026-03-28), no visible releases/tags, no npm stats/downloads/versions, and no dependent packages mentioned. Appears experimental or pre-release. Use with caution for production; suitable for testing CLI banners or build metadata.
Citations:
- 1: https://github.com/teneplaysofficial/echo-banner
- 2: https://registry.npmjs.org/%40tenedev%2Freact-slideshow
echo-banner package is not published on npm and too experimental for production use.
This package was created only 9 days ago with no releases or tags on GitHub and does not appear to be available on npm registry. Using an unpublished or unavailable package in a production workspace breaks dependency resolution. Before using this package, verify:
- It is published to npm with proper versioning and changelog
- Installation succeeds via pnpm install
- It is stable enough for production use
Consider using a stable, published alternative instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pnpm-workspace.yaml` at line 6, The workspace lists an
unpublished/experimental package "echo-banner" in pnpm-workspace.yaml which will
break dependency resolution; remove or replace the echo-banner entry (or pin it
to a valid published version) and instead add a stable published alternative or
a local workspace reference that actually exists; ensure pnpm install succeeds
after updating pnpm-workspace.yaml and verify the package is published on npm
(or convert to a local package folder and reference it via its workspace name)
before reintroducing echo-banner.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
package.json (1)
50-53: Consider adding type declarations forecho-banner.The dependency swap looks correct. However, given that
@types/figletwas removed from devDependencies and the pipeline is failing with TypeScript errors for theecho-bannermodule, you may need to add type declarations ifecho-bannerdoesn't ship its own types.If
echo-bannerdoesn't include TypeScript declarations, you may need to:
- Add
@types/echo-bannerto devDependencies (if available), or- Create a local type declaration file (e.g.,
echo-banner.d.ts)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 50 - 53, TypeScript errors indicate `echo-banner` lacks type declarations; either add a devDependency `@types/echo-banner` if it exists (update devDependencies in package.json) or create a local ambient declaration file (e.g., declare module "echo-banner"; in a new echo-banner.d.ts included by tsconfig) and commit it so the compiler recognizes the module; update tsconfig "typeRoots" or include pattern if necessary to ensure the new .d.ts is picked up.packages/juk-cli/src/index.ts (1)
35-37: Verifyecho-bannerexpectations.The call
print({ pkg })passes an object with apkgproperty. Ensure this matches the expected signature ofecho-banner'sIf the types cannot be resolved, consider adding a minimal type declaration:
Create a type declaration file if echo-banner lacks types
Create
packages/juk-cli/src/echo-banner.d.ts:declare module 'echo-banner' { interface PrintOptions { pkg: { name?: string; version?: string; // Add other properties as needed }; } export function print(options: PrintOptions): Promise<void>; }What is the API for the npm package echo-banner and does it export a print function?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/juk-cli/src/index.ts` around lines 35 - 37, The call await print({ pkg }) may not match the actual echo-banner API and is causing unresolved-type warnings; confirm whether echo-banner exports a print function and its expected parameter shape, then either adjust the call site (the print invocation in packages/juk-cli/src/index.ts) to match the real signature or add a minimal ambient declaration module for 'echo-banner' that declares the exported print function and its PrintOptions (including a pkg with name/version) so TypeScript can resolve the type; create the declaration as a new echo-banner.d.ts in the juk-cli src if the package lacks types and update the call accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@package.json`:
- Around line 50-53: TypeScript errors indicate `echo-banner` lacks type
declarations; either add a devDependency `@types/echo-banner` if it exists
(update devDependencies in package.json) or create a local ambient declaration
file (e.g., declare module "echo-banner"; in a new echo-banner.d.ts included by
tsconfig) and commit it so the compiler recognizes the module; update tsconfig
"typeRoots" or include pattern if necessary to ensure the new .d.ts is picked
up.
In `@packages/juk-cli/src/index.ts`:
- Around line 35-37: The call await print({ pkg }) may not match the actual
echo-banner API and is causing unresolved-type warnings; confirm whether
echo-banner exports a print function and its expected parameter shape, then
either adjust the call site (the print invocation in
packages/juk-cli/src/index.ts) to match the real signature or add a minimal
ambient declaration module for 'echo-banner' that declares the exported print
function and its PrintOptions (including a pkg with name/version) so TypeScript
can resolve the type; create the declaration as a new echo-banner.d.ts in the
juk-cli src if the package lacks types and update the call accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6b73836c-85f9-489e-8baf-71fba8a08d2b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.changeset/common-lands-walk.mdpackage.jsonpackages/juk-cli/package.jsonpackages/juk-cli/src/index.tspackages/juk-cli/src/printBanner.tspnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- packages/juk-cli/src/printBanner.ts
1 participant
Summary by CodeRabbit
Improvements