Conversation
Preview deployments |
Host Test Results 1 files ±0 1 suites ±0 3h 7m 48s ⏱️ + 1h 21m 26s For more details on these errors, see this check. Results for commit 7234df4. ± Comparison against base commit e7401f2. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Pull request overview
This PR optimizes local development and CI by switching from production icons URLs to a locally-served icons server, eliminating external dependencies and improving performance (especially for developers outside the US).
Changes:
- Modified default
iconsURLconfiguration to usehttp://localhost:4206for local-first behavior - Added CORS headers to boxel-icons server for authenticated requests (
Authorization,X-Boxel-Assume-User) - Integrated icons server startup into development and test realm scripts with proper lifecycle management
- Set production
ICONS_URL=https://boxel-icons.boxel.aiin build-host workflow for deployments - Updated test expectations to use localhost icons URLs instead of production URLs
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/boxel-icons/package.json | Added specific CORS headers (Authorization, X-Boxel-Assume-User) to serve command for authenticated icon requests |
| packages/boxel-icons/README.md | Updated documentation to note CORS headers for authenticated fetches |
| packages/host/config/environment.js | Changed default iconsURL from production to http://localhost:4206 |
| packages/host/tests/integration/realm-indexing-test.gts | Updated test expectations to use localhost:4206 instead of production icons URL |
| packages/realm-server/scripts/start-icons.sh | New script to start icons server with idempotent behavior (checks if already running) |
| packages/realm-server/scripts/start-development.sh | Integrated icons server startup with background process management and cleanup |
| packages/realm-server/scripts/start-test-realms.sh | Integrated icons server startup with background process management and cleanup |
| packages/realm-server/scripts/start-without-matrix.sh | Added icons URL to readiness check wait conditions |
| packages/realm-server/scripts/start-all.sh | Added icons URL to readiness check wait conditions |
| packages/realm-server/scripts/start-all-except-optional.sh | Added icons URL to readiness check wait conditions |
| packages/realm-server/scripts/start-services-for-matrix-tests.sh | Added icons URL to readiness check wait conditions |
| packages/realm-server/scripts/start-services-for-host-tests.sh | Added icons URL to readiness check wait conditions |
| packages/realm-server/package.json | Added start:icons npm script |
| .github/workflows/build-host.yml | Set ICONS_URL=https://boxel-icons.boxel.ai for production builds |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
habdelra
left a comment
There was a problem hiding this comment.
looks good, could we also document in the port table of the project readme that we are using 4206 for serving icons?
Currently in CI and for local dev we hit the production icons. While this doesn't seem to save much/any time in CI it's about a 20+% speedup locally for me (I assume due to latency hitting the US from the UK, balanced against a http2 server vs local http1.1), and removes a production requirement for CI and dev.
Tests already started the server, we just never used it.
Local dev built things but wasn't setup to start the server
Summary
iconsURLtohttp://localhost:4206for local-first behavior, matching the other defaultsICONS_URLfromtest-web-assetsand set productionICONS_URL=https://boxel-icons.boxel.aiinbuild-hostenv setupstart-development.shandstart-test-realms.sh, and update orchestrator scripts to rely on those scripts while still waiting forhttp://localhost:4206readinessKey things to review