refactor(icons): extract duplicate utilities, fix naming consistency, improve docs

[Copilot]

The icons package contained ~180 lines of duplicated setup code between generate-manifest.js and vendor-icons.js, inconsistent file naming in documentation, and unclear package structure for new users.

Changes

• Extract shared utilities: Created scripts/shared-utils.js consolidating environment checks, Font Awesome registry configuration, package installation, and common helper functions
• Fix naming inconsistency: Corrected README reference from generateManifest.js to generate-manifest.js
• Improve documentation structure: Added Overview, Installation, and Usage sections upfront; simplified contributing instructions with pnpm commands
• Clarify license: Changed package.json license from "UNLICENSED" to "SEE LICENSE IN LICENSE" referencing the custom license file
• Add explanatory comments: Documented FA_PACKAGES_CONFIG to explain why all packages (Pro, free, custom kit) are installed together

Example

Both scripts now import shared setup:

const {
  PATHS,
  checkEnvironment,
  setupSourceDirectory,
  configureFontAwesomeRegistry,
  installFontAwesomePackages,
} = require("./shared-utils");

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
rivet-chat-room	Error		Jan 7, 2026 11:09pm
rivet-chat-room-jh8c	Canceled		Jan 7, 2026 11:09pm

[claude]

Pull Request Review

This PR successfully refactors the icons package by extracting ~180 lines of duplicate code into a shared utilities module. The refactoring is well-executed overall, with improvements to documentation and naming consistency.

✅ Strengths

1. Excellent DRY principle application: The extraction of shared utilities eliminates significant code duplication between generate-manifest.js and vendor-icons.js
2. Good documentation improvements:
   • Added clear Overview, Installation, and Usage sections
   • Fixed the naming inconsistency (generateManifest.js → generate-manifest.js)
   • Added helpful comments explaining FA_PACKAGES_CONFIG
3. License clarity: Changed from UNLICENSED to SEE LICENSE IN LICENSE, which is more explicit
4. Clean module organization: The shared-utils module is well-structured with clear sections

🐛 Potential Issues

1. Missing type: "module" in package.json for scripts

The scripts use ES modules (import/export) but there is no guarantee that the scripts directory will be treated as ES modules. While the main package.json has "type": "module", the scripts create a temporary package.json in the src directory without this field.

Location: shared-utils.js:125-130

Recommendation: Add "type": "module" to the temporary package.json

2. Inconsistent error handling in main()

In generate-manifest.js:293-297, the error handler does not extract the error message like in vendor-icons.js:285-290. This inconsistency could lead to less helpful error output.

Location: generate-manifest.js:293-297

3. Hardcoded module path resolution

shared-utils.js:19 uses import.meta.dirname which is a relatively new Node.js feature (v20.11.0+). Given that package.json specifies "node": ">=18", this could cause issues with Node.js 18.x.

Location: shared-utils.js:17-20

Recommendation: Use a more compatible approach or update the minimum Node version requirement to 20.11+

💡 Minor Suggestions

1. Code quality: The error() function in shared-utils.js:64 inconsistently includes the emoji inline while log() takes it as a parameter. Consider making them consistent for better code style.

2. Type safety: Consider adding JSDoc type annotations to the exported constants (PATHS, FA_PACKAGES_CONFIG) for better IDE support.

🔒 Security

No security concerns identified. The scripts properly check for required environment variables and only run for maintainers with appropriate credentials.

⚡ Performance

No performance concerns. The refactoring does not change the runtime behavior, only improves maintainability.

🧪 Test Coverage

This is a build-time script package. No tests are expected for these utility scripts, but consider testing the scripts manually after changes:

1. Run pnpm manifest to ensure manifest generation works
2. Run pnpm vendor to ensure icon vendoring works
3. Verify the generated files are valid

📋 Summary

This is a solid refactoring that significantly improves code maintainability. The main concerns are:

1. Critical: Node.js version compatibility (import.meta.dirname requires Node 20.11+)
2. Important: Missing type: "module" in temporary package.json
3. Minor: Inconsistent error handling between scripts

Recommendation: Address the Node.js compatibility issue and the module type issue before merging. The error handling inconsistency is minor but worth fixing for consistency.

---

Review generated by Claude Code

[pkg-pr-new]

More templates

• actor-actions
• example-ai-agent
• example-ai-and-user-generated-actors-freestyle
• chat-room
• chat-room-next-js
• example-cloudflare-workers
• example-cloudflare-workers-hono
• example-cloudflare-workers-inline-client
• cross-actor-actions
• example-cursors
• example-cursors-raw-websocket
• custom-serverless
• example-deno
• example-sqlite
• example-elysia
• example-experimental-durable-streams-ai-agent
• example-express
• example-hono
• hono-bun
• hono-react
• example-kitchen-sink
• multi-region
• native-websockets
• next-js
• example-node
• example-raw-fetch-handler
• example-raw-websocket-handler
• example-raw-websocket-handler-proxy
• example-react
• scheduling
• state
• example-stream
• example-trpc

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@3784

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@3784

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@3784

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@3784

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@3784

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@3784

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@3784

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@3784

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@3784

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@3784

commit: d1ef4ca

[jog1t]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• refactor(icons): extract duplicate utilities, fix naming consistency, improve docs #3784 👈 (View in Graphite)
• refactor(frontend): make building icons process as one-time requirement #3776
• main

---

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.