Skip to content

refactor: icons chunk#4567

Merged
jog1t merged 1 commit intomainfrom
04-06-refactor_icons_chunk
Apr 15, 2026
Merged

refactor: icons chunk#4567
jog1t merged 1 commit intomainfrom
04-06-refactor_icons_chunk

Conversation

@jog1t
Copy link
Copy Markdown
Contributor

@jog1t jog1t commented Apr 6, 2026

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@railway-app
Copy link
Copy Markdown

railway-app bot commented Apr 6, 2026

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

Service Status Web Updated (UTC)
kitchen-sink ❌ Build Failed (View Logs) Web Apr 15, 2026 at 3:11 pm
frontend-cloud 😴 Sleeping (View Logs) Web Apr 15, 2026 at 3:10 pm
frontend-inspector 😴 Sleeping (View Logs) Web Apr 15, 2026 at 3:04 pm
ladle ❌ Build Failed (View Logs) Web Apr 15, 2026 at 2:53 pm
website 😴 Sleeping (View Logs) Web Apr 15, 2026 at 2:51 pm
mcp-hub ✅ Success (View Logs) Web Apr 14, 2026 at 7:46 pm

Copy link
Copy Markdown
Contributor Author

jog1t commented Apr 6, 2026

@jog1t jog1t mentioned this pull request Apr 6, 2026
11 tasks
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Apr 6, 2026

More templates

@rivetkit/cloudflare-workers

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/sqlite-vfs

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sqlite-vfs@4567

@rivetkit/traces

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

@rivetkit/workflow-engine

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/workflow-engine@4567

@rivetkit/virtual-websocket

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: e46d451

@claude
Copy link
Copy Markdown

claude bot commented Apr 6, 2026

Code Review: refactor: icons chunk

Overview

This PR refactors frontend/packages/icons (the @rivet-gg/icons package) from a monolithic bundle to individual per-icon chunk files. The motivation, stated in scripts/vendor-icons.js, is that by bundling FontAwesome Pro SVG data into self-contained dist/icons/fa*.js files and committing them, consumers no longer need a FontAwesome Pro token. The new ./icons/*.js subpath export also enables better tree-shaking.

The diff is enormous (334K additions, 151K deletions) but is entirely generated output from the vendor script, not hand-written code.


Issues

1. Missing dist/index.js in the PR

The vendor script explicitly creates dist/index.js as a barrel re-exporting all icons and the Icon component, and lists it as a required build output. package.json declares "main": "./dist/index.js". However, dist/index.js is not among the PR changes. If it was deleted as part of replacing the old monolithic bundle but not re-committed, the package default entry point is broken for consumers. Please verify and include it.

2. Repeated esbuild boilerplate inflates the repo significantly

Each of the ~3000 icon files is ~67 lines, of which ~58 are identical esbuild CommonJS interop helpers (__create, __defProp, __copyProps, etc.). This is roughly 174K lines of duplicated boilerplate. Consider building with splitting: true and format: 'esm' so esbuild emits a shared chunk file instead. This would substantially reduce the committed diff size on future regenerations.

3. No TypeScript declarations for ./icons/*.js subpath exports

The package.json exports field includes "./icons/*.js" for individual icon imports but has no "types" entry for that subpath. TypeScript consumers doing a subpath import like import { definition } from '@rivet-gg/icons/icons/faActors.js' will get no type information. Either generate .d.ts files per icon or add a wildcard types entry pointing to a declaration stub.

4. Package name uses deprecated @rivet-gg namespace

The package is named @rivet-gg/icons. Per CLAUDE.md, rivet-gg is deprecated. This is a pre-existing issue not introduced by this PR, but since the package is being actively updated this is a good opportunity to rename it with a major version bump.


Observations

  • dist/icon-component.js is a clean extraction of the Icon React wrapper into its own entry point, good for tree-shaking.
  • The vendor script approach is sound: committing generated dist files so consumers need no FA Pro token is well-motivated and documented with legal notices.
  • The PR description is empty with all checklist items unchecked. Please add a brief summary of what changed and why so future contributors understand why 3000 files are committed.

Summary

The chunking approach is architecturally solid. Two blockers before merging:

  1. Confirm dist/index.js is present and correct; add it to this PR if it was inadvertently excluded.
  2. Add TypeScript type coverage for ./icons/*.js subpath exports, or document the gap explicitly.

The boilerplate repetition and namespace issues are non-blocking but worth a follow-up ticket.

@jog1t jog1t force-pushed the 03-25-refactor_frontend_actor_list_uplift branch from ae90870 to ba14ee2 Compare April 14, 2026 19:44
@jog1t jog1t force-pushed the 04-06-refactor_icons_chunk branch from e46d451 to 48aaa25 Compare April 14, 2026 19:44
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-4567 April 14, 2026 19:44 Destroyed
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 14, 2026

Preview packages published to npm

Install with:

npm install rivetkit@pr-4567

All packages published as 0.0.0-pr.4567.4c10a30 with tag pr-4567.

Engine binary is shipped via @rivetkit/engine-cli on linux-x64-musl, linux-arm64-musl, darwin-x64, and darwin-arm64. Windows users should use the release installer or set RIVET_ENGINE_BINARY.

Docker images:

docker pull rivetdev/engine:slim-4c10a30
docker pull rivetdev/engine:full-4c10a30
Individual packages
npm install rivetkit@pr-4567
npm install @rivetkit/react@pr-4567
npm install @rivetkit/rivetkit-native@pr-4567
npm install @rivetkit/sqlite-wasm@pr-4567
npm install @rivetkit/workflow-engine@pr-4567

@jog1t jog1t force-pushed the 04-06-refactor_icons_chunk branch from 48aaa25 to 6c78503 Compare April 15, 2026 14:36
@jog1t jog1t force-pushed the 03-25-refactor_frontend_actor_list_uplift branch from ba14ee2 to d41b1a5 Compare April 15, 2026 14:36
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-4567 April 15, 2026 14:36 Destroyed
@jog1t jog1t marked this pull request as ready for review April 15, 2026 14:44
@jog1t jog1t force-pushed the 04-06-refactor_icons_chunk branch from 6c78503 to d800db9 Compare April 15, 2026 14:52
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-4567 April 15, 2026 14:52 Destroyed
@jog1t jog1t mentioned this pull request Apr 15, 2026
11 tasks
Copy link
Copy Markdown
Contributor Author

jog1t commented Apr 15, 2026

Merge activity

  • Apr 15, 3:02 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Apr 15, 3:11 PM UTC: Graphite rebased this pull request as part of a merge.
  • Apr 15, 3:12 PM UTC: @jog1t merged this pull request with Graphite.

@jog1t jog1t changed the base branch from 03-25-refactor_frontend_actor_list_uplift to graphite-base/4567 April 15, 2026 15:07
@jog1t jog1t changed the base branch from graphite-base/4567 to main April 15, 2026 15:09
@jog1t jog1t force-pushed the 04-06-refactor_icons_chunk branch from d800db9 to 569395b Compare April 15, 2026 15:10
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-4567 April 15, 2026 15:10 Destroyed
@jog1t jog1t merged commit 233a99c into main Apr 15, 2026
26 of 32 checks passed
@jog1t jog1t deleted the 04-06-refactor_icons_chunk branch April 15, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant