refactor(frontend): make building icons process as one-time requirement

[jog1t]

No description provided.

[railway-app]

🚅 Deployed to the rivet-pr-3776 environment in rivet-frontend

Service	Status	Web	Updated (UTC)
frontend-cloud-prod	😴 Sleeping (View Logs)	Web	Jan 9, 2026 at 12:07 am
frontend-cloud-staging	😴 Sleeping (View Logs)	Web	Jan 9, 2026 at 12:06 am
frontend-inspector	😴 Sleeping (View Logs)	Web	Jan 9, 2026 at 12:06 am
website	❌ Build Failed (View Logs)	Web	Jan 8, 2026 at 11:57 pm

[jog1t]

• refactor(icons): extract duplicate utilities, fix naming consistency, improve docs #3784
• refactor(frontend): make building icons process as one-time requirement #3776 👈 (View in Graphite)
• 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.

[graphite-app]

Graphite Automations

"Test" took an action on this PR • (01/07/26)

1 assignee was added to this PR based on Kacper Wojciechowski's automation.

[claude]

PR Review: Refactor Frontend Icons Build Process

Summary

This PR refactors the @rivet-gg/icons package to move icon generation from a postinstall hook to a pre-commit vendoring process. Icons (including Font Awesome Pro) are now pre-generated and committed to the repository, eliminating the need for consumers to have a Font Awesome token.

Strengths

1. Improved Developer Experience: Consumers no longer need FONTAWESOME_PACKAGE_TOKEN, making the package easier to use
2. Better Build Reliability: Eliminates runtime dependencies on Font Awesome registry during installation
3. Clear Separation of Concerns: generate-manifest.js scans FA packages and creates manifest; vendor-icons.js generates icon files from manifest
4. Comprehensive Documentation: README clearly explains the legal restrictions and contribution workflow
5. Proper Licensing: Added LICENSE file and legal notices throughout

Code Quality Observations

Good Practices

• Well-structured scripts with clear phases (setup, generation, bundling)
• Good error handling with descriptive messages
• Helpful logging with emojis for UX
• Duplicate detection to avoid icon conflicts

Areas for Improvement

1. Security Concern - Token Exposure

The generated .npmrc file is properly gitignored, but consider adding verification that it is not being tracked before proceeding with script execution.

2. Missing Error Context (vendor-icons.js:177-190)

When an icon does not exist, it silently falls back to faSquare. This could hide missing icons. Consider logging a warning when fallback occurs so maintainers know which icons failed to resolve.

3. Hardcoded Package Versions

Package versions are duplicated in both scripts. Consider extracting to a shared config file to maintain DRY principle and prevent version mismatches.

4. Bundle Size Impact

The PR adds 79,261 lines to dist/index.js (~104MB total additions). While necessary for pre-vendoring, consider:

• Documenting the expected bundle size in the README
• Using Git LFS for large generated files if repository size becomes problematic
• Verifying tree-shaking works correctly for consumers

5. Type Safety

The @ts-nocheck disables all TypeScript checking. Consider converting to TypeScript for better maintainability or adding complete JSDoc types.

6. Missing Cleanup

Both scripts create temporary files in src/ but do not clean them up. Consider cleanup after success or document why they are kept for caching.

Testing Recommendations

1. Verify icon resolution: Test that commonly used icons resolve correctly and do not fall back to faSquare
2. Test without FA token: Confirm consumers can pnpm install without FONTAWESOME_PACKAGE_TOKEN
3. Bundle size analysis: Verify tree-shaking works and only used icons are bundled
4. Edge cases: Test behavior with corrupted manifest, unavailable FA packages, special character icon names

Minor Issues

1. README line 34 references scripts/generateManifest.js but should be scripts/generate-manifest.js
2. Verify package.json types field pointing to ./src/index.gen.ts works properly for consumers

Performance Considerations

The esbuild configuration looks solid with tree-shaking enabled, neutral platform, ESM format, and external peer dependencies properly configured.

Security Assessment

Low Risk - The scripts only run with explicit maintainer action, require authentication token (not exposed in code), generate static files, and .npmrc is properly gitignored. Consider adding verification that .npmrc is not tracked before running.

Overall Assessment

Recommendation: Approve with minor suggestions

This is a well-executed refactor that significantly improves the developer experience. The code quality is good, with clear structure and helpful documentation. The main concerns are around error visibility (silent fallbacks) and repository size impact, but these are manageable.

The legal licensing is properly handled, and the separation of manifest generation from vendoring is a good architectural choice.

Priority Actions:

1. Add warning logs when icons fall back to faSquare
2. Document expected bundle/repository size impact
3. Test tree-shaking works correctly for consumers
4. Consider cleanup of temporary files or document why they are kept

Great work on this refactor!

[jog1t]

hey @copilot can you:

• fix the 1. Duplicate Code (DRY violation)
• fix the 1. Inconsistent File Naming (scripts/generate-manifest.js:34)
• fix the 4. README Improvements
• fix the Line 5: "license": "UNLICENSED" contradicts the LICENSE file., provide correct value here
• fix the 1. Duplicate Code (DRY violation)

[Copilot]

@jog1t I've opened a new pull request, #3784, to work on those changes. Once the pull request is ready, I'll request review from you.

[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@3776

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/virtual-websocket

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: 00601f7

[claude]

PR Review: refactor(frontend): make building icons process as one-time requirement

Overview

This PR refactors the @rivet-gg/icons package to pre-generate and commit all Font Awesome icons (including Pro icons) to the repository. This eliminates the need for consumers to have a Font Awesome Pro token.

✅ Strengths

Architecture & Design

• Excellent separation of concerns between maintainer workflows and consumer usage
• Well-structured scripts that are modular and follow single-responsibility principle
• Smart deduplication handling icon aliases with registeredIcons Set
• Proper fallback handling to faSquare for missing icons

Code Quality

• Clear documentation with legal notices, troubleshooting, and maintainer instructions
• Good error handling with environment variable checks
• Proper LICENSE file protecting Font Awesome Pro licensing

User Experience

• Zero-config for consumers - no more postinstall scripts or token requirements
• Faster installs - no icon processing during pnpm install
• Reliable - eliminates build-time failures from missing tokens

🔍 Issues & Concerns

1. Large Binary Files in Git (CRITICAL)

The PR commits a 3.5MB dist/index.js file to git:

• dist/index.js: 3.5MB (79,261 lines)
• manifest.json: 424KB
• src/index.gen.js: ~200KB
• src/index.gen.ts: ~200KB

Problems: Bloats repository size permanently, every clone downloads this data, diffs are meaningless for generated code.

Recommendations:

1. Consider Git LFS for dist/index.js
2. Alternative: Build dist/index.js on npm publish via prepublish script
3. Update .gitattributes to mark generated files

2. Missing Input Validation

Location: shared-utils.js:83-87

The faCamelCase function lacks validation that str is defined. Could crash on edge cases.

3. Error Handling Inconsistency

Location: vendor-icons.js:240-243

Stack traces are lost in bundleWithEsbuild error handler. The pattern at vendor-icons.js:285-292 is correct - just inconsistent.

4. Security: Environment Variable Spreading (LOW)

Location: shared-utils.js:143-147

The env: { ...process.env } spreads all environment variables into npm subprocess. Be explicit about which vars to pass (PATH, HOME, FONTAWESOME_PACKAGE_TOKEN).

5. Missing Cleanup

Scripts create src/node_modules, src/.npmrc, src/package.json but never clean them up. Document cleanup process for maintainers.

6. TypeScript Type Definitions

Location: package.json:32

TypeScript expects .d.ts files, not .ts source files. This is unconventional and could cause issues.

7. Performance: O(n²) Loops

Location: generate-manifest.js:83-107

Two-pass algorithm is fine for ~5000 icons but could be optimized with Map for O(1) lookups if icon count grows.

🎯 Testing Recommendations

Missing test coverage:

1. Unit tests for utility functions (faCamelCase, etc.)
2. Integration tests for manifest → vendor → bundle pipeline
3. Edge case testing: missing icons, duplicates, invalid manifest

📊 Performance Considerations

Positive:

• Faster consumer installs
• Smaller consumer node_modules
• Tree-shaking friendly ESM exports

Concerns:

• 3.5MB bundle size concerning for browser bundles
• Verify tree-shaking works correctly in consumer apps

🔒 Security Review

Good practices:

• No dynamic code execution
• Safe use of spawnSync with array arguments
• Environment variable validation
• No credential exposure

Minor concerns: See issue #4 above.

📝 Documentation Suggestions

Add to CLAUDE.md:

• Document that generated files are committed to git
• Maintainer-only: Run pnpm manifest then pnpm vendor to update icons
• Never run icon generation in CI or consumer environments
• Note 3.5MB dist/index.js size

✅ Checklist

• Code follows repository conventions
• Proper error handling
• Legal/licensing considerations addressed
• Documentation updated
• No security vulnerabilities
• ⚠️ Large binary files consideration
• Tests added (missing)
• TypeScript types correct

🎬 Recommendation

Approve with minor changes requested.

The refactor is well-designed and significantly improves developer experience. Main concern is the large generated file in git.

Priority actions:

1. Before merging: Decide on strategy for dist/index.js (Git LFS, prepublish build, or document why it is acceptable)
2. Low priority: Address input validation and error handling consistency
3. Future work: Add test coverage and review TypeScript type definitions

The code quality is high and architectural improvements are significant. Great work! 🎉

---

Review completed by Claude Code 🤖

[NathanFlurry]

put this in to smth like gen:vendor and gen:manifest then add a gen script that does both

[claude]

Pull Request Review: refactor(frontend): make building icons process as one-time requirement

Overview

This PR refactors the Rivet Icons package to vendor all Font Awesome icons (including Pro icons) directly into the repository. This is a significant architectural change that shifts from a build-time icon generation model to a pre-generated/committed model.

Summary of Changes

Problem Solved: Previously, the icons package required each consumer to have FONTAWESOME_PACKAGE_TOKEN set during pnpm install to generate icons via postinstall scripts. This PR eliminates that requirement by committing pre-generated icon files to the repository.

Key Changes:

• ✅ New LICENSE file with clear legal restrictions
• ✅ Pre-generated icon files committed to git (dist/index.js, src/index.gen.js, src/index.gen.ts)
• ✅ Removed postinstall scripts - package now works out-of-the-box
• ✅ New maintainer-only vendor scripts (generate-manifest.js, vendor-icons.js, shared-utils.js)
• ✅ Updated documentation explaining the new workflow
• ✅ Updated .gitignore and .dockerignore to include the dist/ directory

Code Quality Assessment

✅ Strengths

1. Well-structured scripts: The new vendor scripts are cleanly organized with clear separation of concerns:

   • shared-utils.js - Common utilities
   • generate-manifest.js - Manifest generation
   • vendor-icons.js - Icon file generation

2. Clear legal notices: Comprehensive LICENSE file and legal banners in generated files make the usage restrictions explicit.

3. Excellent documentation: README is significantly improved with clear distinction between user and maintainer workflows.

4. Good error handling: Scripts check for required environment variables and provide helpful error messages.

5. Deduplication logic: Smart handling of icon aliases and preventing duplicate exports.

⚠️ Issues and Concerns

1. Repository Size Impact (HIGH)

Committing 79,261 lines of generated JavaScript significantly increases repository size:

• Current state: Adds ~1.5MB+ of code to the repo
• Long-term: Every icon update will create large diffs
• Recommendation: This is acceptable for a monorepo, but worth documenting

2. Missing Build Verification (MEDIUM)

The package.json has scripts but no way to verify committed files are up-to-date. Recommendation: Add a CI verification script to ensure generated files match what the scripts would produce.

3. Icon Fallback Behavior Unclear (MEDIUM)

In vendor-icons.js:69-82, when an icon doesn't exist, it silently falls back to faSquare. This could mask problems where icon names are misspelled in the manifest, expected Pro icons aren't available, or package versions change.

Recommendation: Add warning logs for fallbacks so maintainers can catch issues early.

4. Hard-coded Icon Versions (LOW)

In shared-utils.js:38-47, versions are hard-coded. Recommendation: Consider using a config file, but this is acceptable for maintainer-only scripts.

5. TypeScript Type Generation (LOW)

The IconProp type is defined manually rather than imported from Font Awesome types. Recommendation: Consider importing from @fortawesome/fontawesome-svg-core to avoid drift.

Security Considerations

✅ No Security Issues Found

1. Environment variable handling: Properly checks for FONTAWESOME_PACKAGE_TOKEN
2. No credential exposure: Token only used in temporary .npmrc (gitignored)
3. Subprocess execution: Uses spawnSync with explicit arguments, no shell injection risk
4. File operations: All paths use join() for proper construction

⚠️ Legal/Licensing Concerns

1. NPM publication: Package has "private": false. Ensure Font Awesome's licensing terms allow this distribution model.

2. Verification: Confirm with Font Awesome that committing Pro icons to a private monorepo for internal use is compliant.

Performance Considerations

✅ Improvements

1. Eliminates build-time overhead: No more postinstall scripts = faster pnpm install
2. Tree-shaking enabled: esbuild config includes proper optimization
3. External dependencies: React and FA packages properly externalized

⚠️ Concerns

1. Bundle size: Large dist/index.js. Consumers should verify tree-shaking works effectively.
2. Git performance: 79K lines will slow git operations slightly (acceptable for monorepo).

Test Coverage

❌ Missing Tests

Critical Gap: No tests for the vendor scripts themselves.

Recommendations:

1. Add tests for icon deduplication logic
2. Add tests for fallback behavior
3. Add CI check to verify committed files match generated output

Recommendations Summary

Must Fix (Before Merge)

None - code is production-ready.

Should Fix (High Priority)

1. Add warning logs for icon fallbacks to faSquare
2. Add CI verification that generated files are up-to-date
3. Confirm Font Awesome Pro license compliance for this distribution model

Nice to Have (Low Priority)

1. Add tests for vendor scripts
2. Document bundle size impact in README
3. Consider importing IconProp type from FA
4. Add metrics about which icons are actually used

Conclusion

This is a well-executed refactoring that solves a real problem (requiring FA token for all consumers). The code quality is high, documentation is excellent, and the approach is sound.

Recommendation: APPROVE with suggestions

The code can be merged as-is, but implementing the "Should Fix" items (especially icon fallback warnings and CI verification) would make the solution more robust.

Key Strengths

• ✅ Eliminates build-time complexity for consumers
• ✅ Clear legal documentation
• ✅ Well-structured maintainer scripts
• ✅ Excellent documentation

Areas for Future Improvement

• 🔄 Add automated verification
• 🔄 Add warning logs for fallbacks
• 🔄 Consider test coverage
• 🔄 Monitor repository size over time

Great work on this refactoring! The approach of vendoring icons is a pragmatic solution that significantly improves the developer experience.