new: favicon package for Solid 2.0#1001
Open
davedbase wants to merge 11 commits into
Open
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🦋 Changeset detectedLatest commit: 853a439 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
davedbase
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
packages/favicon/src/canvas.ts (1)
8-15: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider
img.crossOrigin = "anonymous"for remote icons.Without it, a cross-origin base icon taints the canvas and
toDataURLthrowsSecurityErrorlater inrenderBadgeIcon/renderProgressIcon. Setting it opt-in (or via an option) lets CORS-enabled hosts work; combined with the rejection handling suggested below, non-CORS hosts degrade to the plain href.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/favicon/src/canvas.ts` around lines 8 - 15, Update loadImage to set the created image element’s crossOrigin property to "anonymous" before assigning img.src, enabling CORS for remote icons while preserving the existing load and rejection behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/favicon/src/animation.ts`:
- Around line 182-191: Update the frames effect callback to resume playback when
the list grows from zero or one frame to multiple frames and autoplay is
enabled, while preserving the existing pause behavior when shrinking to one or
fewer frames. Use the existing playing state and play/pause controls in the
createEffect handler around setFrame and favicon.setHref.
In `@packages/favicon/src/badge.ts`:
- Around line 106-108: Update makeFavicon in packages/favicon/src/badge.ts at
lines 106-108 and makeFaviconProgress in packages/favicon/src/progress.ts at
lines 82-84 to guard asynchronous render completion: preserve the base href on
rejection, prevent setHref after disposal, and return a wrapped controller whose
dispose marks it disposed. Prefer extracting this shared guard into a helper in
packages/favicon/src/canvas.ts alongside createCanvasFavicon.
In `@packages/favicon/src/canvas.ts`:
- Around line 72-94: Update redraw to handle rejected render promises by
catching failures without producing unhandled rejections, and invalidate pending
renders during onCleanup so settled draws cannot call favicon.setHref or
setCurrent after disposal. Reuse the existing requestId guard by advancing or
otherwise invalidating requestId in cleanup before calling favicon.dispose.
In `@packages/favicon/src/link.ts`:
- Line 26: Update the previousHref capture in the link reuse logic to read the
raw href attribute rather than HTMLLinkElement.href, preserving undefined when
the attribute is absent. Ensure the restoration logic around previousHref does
not write an empty string back, so reused attribute-less link elements remain
without an href.
In `@packages/favicon/test/badge.test.ts`:
- Around line 101-109: Update the test around createFaviconBadge to retain and
invoke the createRoot disposal callback after asserting the initial badge, then
verify href is restored to baseHref. Ensure cleanup runs within the test so the
reactive root does not leak.
In `@packages/favicon/test/progress.test.ts`:
- Around line 95-103: Update the “applies the initial progress and restores on
cleanup” test to retain the disposer returned by createRoot and invoke it after
asserting the initial favicon progress, then flush pending microtasks and assert
the cleanup restoration behavior. Ensure the root is explicitly disposed so the
registered onCleanup handler executes.
In `@packages/favicon/test/setup.ts`:
- Around line 98-115: Register the favicon test setup module in the shared jsdom
Vitest setupFiles configuration so it runs before source-module imports and
activates window.matchMedia early. Update the setup around the
HTMLImageElement.prototype src mock to assert that the fake matchMedia and
synthetic image-load state are installed successfully.
In `@patches/storybook-solidjs-vite`@10.5.2.patch:
- Around line 10-24: Correct the TypeScript module import paths in the Storybook
SolidJS Vite patch: change both the static `ts` import and the dynamic import
inside `getOrCreateSolidComponentMetaManager` from four parent traversals to
three, targeting the stable `typescript@6.0.3` store path.
In `@template/stories/tsconfig.json`:
- Line 2: Update the extends path in the template/stories tsconfig to reference
the repository’s .storybook/tsconfig.json from the template/stories directory,
removing the extra parent traversal. Preserve the existing Storybook
configuration target.
---
Nitpick comments:
In `@packages/favicon/src/canvas.ts`:
- Around line 8-15: Update loadImage to set the created image element’s
crossOrigin property to "anonymous" before assigning img.src, enabling CORS for
remote icons while preserving the existing load and rejection behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 473766a4-bead-47f9-b082-fd27098f8c1d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
.changeset/favicon-initial.mdpackages/favicon/LICENSEpackages/favicon/README.mdpackages/favicon/package.jsonpackages/favicon/src/animation.tspackages/favicon/src/badge.tspackages/favicon/src/canvas.tspackages/favicon/src/components.tsxpackages/favicon/src/favicon.tspackages/favicon/src/index.tspackages/favicon/src/link.tspackages/favicon/src/progress.tspackages/favicon/src/scheme.tspackages/favicon/stories/_helpers.tsxpackages/favicon/stories/createFavicon.stories.tsxpackages/favicon/stories/createFaviconAnimation.stories.tsxpackages/favicon/stories/createFaviconBadge.stories.tsxpackages/favicon/stories/createFaviconProgress.stories.tsxpackages/favicon/stories/createFaviconScheme.stories.tsxpackages/favicon/stories/tsconfig.jsonpackages/favicon/test/animation.test.tspackages/favicon/test/badge.test.tspackages/favicon/test/components.test.tsxpackages/favicon/test/favicon.test.tspackages/favicon/test/progress.test.tspackages/favicon/test/scheme.test.tspackages/favicon/test/server.test.tsxpackages/favicon/test/setup.tspackages/favicon/tsconfig.jsonpatches/storybook-solidjs-vite@10.5.2.patchpnpm-workspace.yamltemplate/stories/tsconfig.json
…n handler or a disposal guard
…ad and an attribute-less <link> gets restored to the page URL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Primitives for controlling the document favicon, built for Solid 2.0 (beta.24).
What's in it
makeFavicon/createFavicon— the base primitive: swap the favicon<link>href, restore the previous one on cleanup.makeFaviconAnimation/createFaviconAnimation— cycle a sequence of hrefs on an interval (build-status spinners, "recording" pulses), auto-pausing while the tab is hidden.makeFaviconBadge/createFaviconBadge— composite a notification count/dot onto a base icon via canvas.makeFaviconScheme/createFaviconScheme— swap between a light/dark icon to trackprefers-color-scheme.makeFaviconProgress/createFaviconProgress— composite a progress ring onto a base icon, for upload/download-style indicators.FaviconLink— a Solid component that renders the favicon<link>directly, so its href is part of the server-rendered HTML rather than only applied post-hydration; composes automatically with any of the reactive primitives above via a shared "reuse existing link" lookup.Each capability is a small, focused
make*/create*pair rather than one kitchen-sinkcreateAdvancedFavicon— consistent with this repo's existing layered-primitive packages (e.g.video). Full reasoning inDESIGN.md.Notes
isServer-guarded) and integrate with<Loading>/hydration the normal way;FaviconLinkcloses the remaining gap by letting the initial favicon ship in the server-rendered HTML itself — see the README for SolidStart/Astro integration examples.tsc/lint/build all clean.Summary by CodeRabbit
New Features
@solid-primitives/faviconpackage.FaviconLinksupport for initial document head rendering.Documentation