chore: replace astro-icon with nimbus-docs Icon component - #32647
Open
mvvmm wants to merge 6 commits into
Open
Conversation
Tests the astro-icon replacement PR in cloudflare/nimbus#76. Uses pkg.pr.new preview build from commit acfac20.
Contributor
ReviewReviewing new changes (commit
👉 Fix in your agent 👈Fix the following review findings in PR #32647 (https://github.com/cloudflare/cloudflare-docs/pull/32647).
Before making changes, review each finding and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g. the fix requires disproportionate effort for minimal benefit,
or the finding is factually incorrect), explain why
- If you need clarification before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order
After triaging, follow this order:
1. Post a comment on this PR for any findings you are skipping, with the finding ID and your reasoning.
2. Then commit the fixes for the legitimate findings.
The comment must come before the commit — the bot reads PR comments when a new
push triggers a review, so skip comments posted after the push will be missed.
---
## Code Review
### Warnings (2)
#### CR-4fac1dfab0a3 · Unstable dependency source
- **File:** `package.json` line 56
- **Issue:** @cloudflare/nimbus-docs is resolved from https://pkg.pr.new/@cloudflare/nimbus-docs@76, a PR preview build, instead of a pinned npm registry version.
- **Fix:** Use a published, semver-pinned npm release of @cloudflare/nimbus-docs for production, or keep this change only as a temporary local/CI testing branch and do not merge it to production.
#### CR-f3a1cb31c3e5 · Supply chain risk
- **File:** `package.json` line 56
- **Issue:** Using a third-party PR preview service (pkg.pr.new) for a core framework dependency bypasses npm registry integrity checks and may pull in unreviewed or changing code every install.
- **Fix:** Consume @cloudflare/nimbus-docs from the npm registry with a pinned version and integrity hash so installs are reproducible and verified.
Code ReviewThis code review is in beta and may not always be helpful — use your judgment. Warnings (2)
ConventionsNo convention issues found. Style Guide ReviewNo style-guide issues found. CommandsOnly codeowners can run commands. Post a comment with the command to trigger it.
|
Contributor
|
This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:
|
mvvmm
marked this pull request as draft
August 10, 2026 20:30
mvvmm
changed the title
mvvmm
force-pushed
the
chore/nimbus-icon-replacement
branch
from
August 10, 2026 20:37
94e2d42 to
ae599f2
Compare
- Replace all astro-icon imports with @cloudflare/nimbus-docs/components/Icon.astro - Collapse AstroIcon alias to Icon (they were the same component) - Update virtual:astro-icon to virtual:nimbus/icons in directory.astro - Remove astro-icon from astro.config.ts and package.json - Point @cloudflare/nimbus-docs at PR #76 preview build
mvvmm
force-pushed
the
chore/nimbus-icon-replacement
branch
from
August 10, 2026 20:44
ae599f2 to
7850a5d
Compare
Contributor
mvvmm
marked this pull request as ready for review
August 11, 2026 19:09
mvvmm
requested review from
a team,
Refaerds,
celso,
ethulia,
jonnyparris,
kathayl,
kflansburg,
kodster28,
mchenco,
meddulla,
omarmosid,
ruifigueira and
superhighfives
as code owners
August 11, 2026 19:09
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.
What
Replaces
astro-iconwith the built-in icon system from cloudflare/nimbus#76 (commitacfac20).Changes
astro-iconfrompackage.json,astro.config.ts, and all 53 component/page filesimport { Icon } from "astro-icon/components"→import Icon from "@cloudflare/nimbus-docs/components/Icon.astro"AstroIconalias —IconandAstroIconwere the same component; merged all usage toIconvirtual:astro-icon→virtual:nimbus/iconsindirectory.astro@ts-expect-error— the nimbus integration injects types forvirtual:nimbus/iconsWhy
astro-iconstamps alastModifiedtimestamp into its virtual module on every build, invalidating Astro's incremental build cache for thousands of pages. The package hasn't had a commit in 2+ years. Nimbus#76 replaces it with a built-in Vite plugin that omitslastModified, making the cache stable.The nimbus PR also includes a workaround for the css-tree
createRequirebug (svgo#2149) that breaks Vite prerendering — it redirectscss-treeandcssoto their browser bundles which have JSON data inlined.Verification
pnpm run check— 0 errors, 0 warnings, 0 hintspnpm run lint— cleanpnpm run format:core:check— cleanAfter merge of nimbus#76
Revert the
@cloudflare/nimbus-docsdependency to the published version (remove thepkg.pr.newURL).