Skip to content

feat: bundle full favicon set as AppShell default#378

Merged
erickteowarang merged 4 commits into
mainfrom
fix/update-favicon-assets
Jul 13, 2026
Merged

feat: bundle full favicon set as AppShell default#378
erickteowarang merged 4 commits into
mainfrom
fix/update-favicon-assets

Conversation

@erickteowarang

@erickteowarang erickteowarang commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

What

AppShell previously shipped a single 32×32 Tailor mark as its bundled default favicon. It now bundles a small, purpose-fit favicon set and injects the whole list when the app mounts:

  • favicon-16x16.png / favicon-32x32.png — the browser-tab icons
  • apple-touch-icon.png (180×180) — home-screen bookmark icon for both iOS and Android

Each asset is embedded as a base64 data URI, so the set ships in the JS bundle with no asset-copy step in consuming apps.

Deliberately excluded

This is a web app framework, not a PWA, so the following are omitted to keep the bundle small (~14 KB vs ~64 KB for the full favicon_io set):

  • favicon.ico — legacy fallback; React 19 doesn't support the browsers that need it, and since we inject a data URI (not a file at /favicon.ico), the browser's auto-fetch never triggers.
  • android-chrome-192/512 + site.webmanifest — only consumed via a web manifest for PWA install / Android home-screen. We don't inject a manifest. Android "Add to Home Screen" falls back to the apple-touch-icon, which is kept.

How

  • src/lib/default-favicon.ts — a typed DEFAULT_FAVICONS list (data URIs + rel/type/sizes).
  • src/components/document-head.tsxDocumentHead renders the whole DEFAULT_FAVICONS list of <link> tags by default (was one).
  • src/lib/favicons/ — raw source files kept for regeneration (in src/, not published — only dist/** ships).
  • scripts/gen-favicons.mjs — regenerates the data-URI module from the raw files (node scripts/gen-favicons.mjs).

Backward compatibility

Only the default changed (one → three). No public API break (DEFAULT_FAVICON_HREF was internal-only).

  • Passing favicon="…" still replaces the whole set with that single href.
  • An existing host-page <link rel="icon"> is still respected (nothing injected).

Testing

  • pnpm type-check ✅ · pnpm lint ✅ · pnpm test ✅ (1314 core tests, incl. updated document-head tests) · pnpm fmt
  • Verified every embedded data URI round-trips to byte-identical source images.

🤖 Generated with Claude Code

AppShell previously shipped a single 32x32 Tailor mark as its bundled
default favicon. It now bundles the full favicon_io set (a multi-resolution
.ico, 16/32/192/512 PNGs, and a 180x180 Apple touch icon), each embedded as
a base64 data URI so the whole set ships in the JS bundle with no asset-copy
step. DocumentHead injects the entire list of <link> tags when no favicon
prop is passed and the host page declares no <link rel="icon">.

Backward compatible: a favicon prop still replaces the whole set with that
single href, and an existing host-page favicon is still respected.

Raw source files live in src/lib/favicons/ (not published); regenerate the
data-URI module with scripts/gen-favicons.mjs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@erickteowarang erickteowarang requested a review from a team as a code owner July 11, 2026 08:23
Swap the six favicon_io source images for the new set and regenerate the
data-URI module. Behavior is unchanged; only the icon artwork differs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@IzumiSy IzumiSy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit expensive for what is essentially a fallback path. Since DocumentHead is part of the root route, the full base64 favicon set currently ends up in the main app-shell bundle for every consumer, even when they already provide their own favicon or have a host-page icon.

In my local production build, dist/app-shell.js went from roughly 375 kB to 438 kB raw and from 86 kB to 122 kB gzip (about +36 kB gzip), which feels fairly noticeable for a default favicon fallback.

Would it make sense to move the full set behind a dedicated subpath export (for example @tailor-platform/app-shell/favicons) and only load it via dynamic import() when neither a favicon prop nor a host-page icon is present? That would preserve the current default behavior while keeping the main bundle a bit leaner.

Drop the legacy .ico and the PWA android-chrome-192/512 icons from the
bundled default set, keeping only the 16/32 PNG tab icons and the 180x180
Apple touch icon. This shrinks the embedded data-URI module from ~67 KB to
~14 KB.

The dropped icons earned nothing on a non-PWA web app: React 19 doesn't
support the browsers that need the .ico, and the android-chrome icons are
only referenced via a web manifest (which we don't inject). The Apple touch
icon still covers "Add to Home Screen" on both iOS and Android.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@erickteowarang

erickteowarang commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Would it make sense to move the full set behind a dedicated subpath export (for example @tailor-platform/app-shell/favicons) and only load it via dynamic import() when neither a favicon prop nor a host-page icon is present? That would preserve the current default behavior while keeping the main bundle a bit leaner.

Good shout out, my issue with lazy-loading all of the favicons is that:

  • It's not SSR-friendly if we ever go down that path
  • Users will see a flash of the globe icon before the favicon kicks in

I do think there's value in deferring the apple-touch icon (because that one is only used for bookmarks in iOS/Safari, which is post-load), so I'll do that, but for the main favicons, they're like ~2KB combined so I don't think pre-optimising for that is worth the effort.

EDIT: After trying it out, I don't think it's worth adding complexity to chunk/lazy-load just the apple-touch favicon, as it's only 7.45 KB zipped. If you think it's worth it let me know.

@IzumiSy

IzumiSy commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Re-checking the latest revision, this is much smaller than when I left the original comment, so I’m happy to retract that concern. Bundling the full set inline seems fine to me now.

@erickteowarang erickteowarang merged commit 0817cf1 into main Jul 13, 2026
4 checks passed
@erickteowarang erickteowarang deleted the fix/update-favicon-assets branch July 13, 2026 04:52
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.

2 participants