Skip to content

fix: unbreak admin-app build after SLASHED codec/lucide renames - #135

Merged
jackgranatowski merged 2 commits into
claude/pr-469-audit-rebase-ggp0e4from
claude/plugins-hotfix-fa-rename
Jul 2, 2026
Merged

fix: unbreak admin-app build after SLASHED codec/lucide renames#135
jackgranatowski merged 2 commits into
claude/pr-469-audit-rebase-ggp0e4from
claude/plugins-hotfix-fa-rename

Conversation

@jackgranatowski

Copy link
Copy Markdown
Contributor

Summary

  • SLASHED's SL-016 (codec.ts's obfuscated fa renamed to generateCSS) and SL-022 (lucide-svelte@lucide/svelte) landed in main via #474.
  • AppOverlay.svelte and admin-app/package.json are plugin-specific / not vendored, so they kept referencing the old names.
  • CI's prebuild hook always re-syncs admin-app/src/ fresh from slashed@main (see the "fetch src/App.svelte" lines in CI logs — there's no local sibling checkout in CI), so this broke vite build on every open plugin PR, not just the one that surfaced it (fix(color-model): filter -dark source tokens consistently across Bricks/Gutenberg (PL-013/014/015) #134's "Build admin-app" check).

Fix

  • AppOverlay.svelte: fagenerateCSS (call site + import), lucide-svelte@lucide/svelte (icon import).
  • admin-app/package.json / package-lock.json: swap the lucide-svelte dependency for @lucide/svelte@^1.23.0.

Type

  • fix

Checklist

  • Conventional Commit messages
  • npm run build (admin-app) verified green locally against the current slashed@main content (auto-detected local sibling checkout), reproducing and then resolving the exact [MISSING_EXPORT] "fa" / unresolved @lucide/svelte failures seen in CI
  • Built SPA assets (assets/admin-app/app.js/app.css) — intentionally not regenerated here. This repo's committed admin-app/src/ vendored copy still predates SL-016/017/018/019/020/022 (that catch-up sync is the separate, already-planned PR-SYNC job). CI never builds from the committed copy anyway — prebuild always re-fetches from slashed@main fresh — so this fix is scoped to only the two plugin-owned files that don't get re-synced.

Notes

Out of scope for this hotfix, tracked separately: the full vendored-source catch-up sync (PR-SYNC), which will also refresh the committed assets/admin-app/ build output.


Generated by Claude Code

…names

SLASHED's SL-016 (codec.ts's `fa` → `generateCSS`) and SL-022
(lucide-svelte → @lucide/svelte) landed in main via #474. AppOverlay.svelte
is plugin-specific and not vendored, so it kept referencing the old names
and broke `vite build` as soon as CI's prebuild sync pulled the renamed
exports from slashed@main — failing on every open plugin PR, not just the
one that triggered it.

Swaps the import/call site to generateCSS and updates package.json/
package-lock.json to depend on @lucide/svelte instead of the deprecated
lucide-svelte package.
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c66c90d0-0a5e-4339-b928-fd948470e4bb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/plugins-hotfix-fa-rename

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix admin-app build after codec and lucide package renames

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Update admin-app overlay to use upstream generateCSS export instead of fa.
• Switch icon imports/dependency from lucide-svelte to @lucide/svelte.
• Restore CI vite build by aligning plugin-owned files with synced slashed@main source.
Diagram

graph TD
  CI["CI prebuild"] --> Sync["Sync slashed src"] --> Source["admin-app/src"] --> Vite["vite build"]
  Source --> Overlay["AppOverlay.svelte"] --> Codec["lib/codec (generateCSS)"]
  Overlay --> Lucide["@lucide/svelte"]
  Pkg["package.json"] --> Lucide
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Add a local compatibility shim (re-export `fa` → `generateCSS`)
  • ➕ Avoids touching call sites across plugin-owned code
  • ➕ Provides a single upgrade point for future upstream renames
  • ➖ Adds indirection/technical debt
  • ➖ Can mask upstream API changes and delay necessary updates
2. Pin the synced slashed source to a known-good commit/tag
  • ➕ Stabilizes builds against upstream breaking changes
  • ➕ Reduces firefighting from unexpected renames
  • ➖ Requires versioning/lock mechanism in the sync step
  • ➖ Delays upstream fixes/features and increases drift

Recommendation: The PR’s approach (update imports + dependency to match upstream) is the most direct and lowest-maintenance fix for a breaking rename. A compatibility shim or pinning could reduce future breakage, but both add ongoing complexity; consider those only if upstream breakages become frequent.

Files changed (2) +4 / -4

Bug fix (1) +3 / -3
AppOverlay.svelteUpdate codec and icon imports to match upstream renames +3/-3

Update codec and icon imports to match upstream renames

• Replaces 'fa(...)' usage with 'generateCSS(...)' and updates the corresponding import from './lib/codec'. Also switches lucide icon imports from 'lucide-svelte' to '@lucide/svelte' to match the updated dependency.

SLASHED-for-WP/admin-app/src/AppOverlay.svelte

Other (1) +1 / -1
package.jsonReplace 'lucide-svelte' with '@lucide/svelte' dependency +1/-1

Replace 'lucide-svelte' with '@lucide/svelte' dependency

• Updates admin-app dependencies to use '@lucide/svelte@^1.23.0' and removes the deprecated 'lucide-svelte' package. Aligns installation with upstream package rename so icon imports resolve during builds.

SLASHED-for-WP/admin-app/package.json

jackgranatowski pushed a commit that referenced this pull request Jul 2, 2026
PR-C1's branch forked before #135 landed, so it inherited the same
stale fa/lucide-svelte references breaking `vite build`.
@qodo-code-review

qodo-code-review Bot commented Jul 2, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 1 rule

Grey Divider


Remediation recommended

1. Admin check script breaks ✓ Resolved 🐞 Bug ☼ Reliability
Description
SLASHED-for-WP/admin-app/package.json removes lucide-svelte but multiple committed files in
admin-app/src/ still import 'lucide-svelte', so npm run check will fail in a fresh checkout
unless the user manually runs npm run sync first. This is inconsistent with dev/build, which
run the sync automatically via predev/prebuild.
Code

SLASHED-for-WP/admin-app/package.json[R13-19]

    "check": "svelte-check --tsconfig ./tsconfig.json"
  },
  "dependencies": {
+    "@lucide/svelte": "^1.23.0",
    "fflate": "^0.8.3",
-    "lucide-svelte": "^1.0.1",
    "motion": "^12.23.24"
  },
Relevance

⭐⭐ Medium

Team cares about CI/dev script reliability, but no prior accepted/rejected change about making
check run sync/precheck.

PR-#27
PR-#18
PR-#122

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The admin-app check script does not run the sync step, but the currently committed vendored source
still imports lucide-svelte while package.json only declares @lucide/svelte, which will cause
module resolution failures during type-checking in a fresh install.

SLASHED-for-WP/admin-app/package.json[7-19]
SLASHED-for-WP/admin-app/src/App.svelte[1-12]
SLASHED-for-WP/admin-app/src/components/shell/SidebarNav.svelte[1-6]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`npm run check` runs `svelte-check` without first syncing `admin-app/src/`, but the repo’s currently committed `src/` still imports `lucide-svelte` while the dependency has been removed. This breaks local type-checking (and any automation that runs the check script) on a fresh install.

### Issue Context
- `dev` and `build` are safe because they run `scripts/sync-core.mjs` via `predev`/`prebuild`.
- `check` has no corresponding `precheck` hook, and the committed `src/` still contains `lucide-svelte` imports.

### Fix Focus Areas
Choose one:
1) **Hotfix-friendly:** Re-add `lucide-svelte` alongside `@lucide/svelte` (until the planned vendored-source catch-up sync lands).
2) **Behavioral consistency:** Add a `precheck` hook (or change `check` to run sync first) so `npm run check` operates on the synced source tree.

- SLASHED-for-WP/admin-app/package.json[7-19]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread SLASHED-for-WP/admin-app/package.json
npm run check (svelte-check) had no equivalent of dev/build's
predev/prebuild sync hook, so it type-checked whatever was already on
disk. On a fresh checkout with the committed vendored src/ still
predating the lucide-svelte rename, that meant a stale module-resolution
error instead of an accurate check against the current framework source.
@jackgranatowski
jackgranatowski merged commit d80e05b into claude/pr-469-audit-rebase-ggp0e4 Jul 2, 2026
9 checks passed
jackgranatowski pushed a commit that referenced this pull request Jul 2, 2026
Brings in the already-merged #134/#135/#136/#138 content; the only
real conflict was two independent additions to CLAUDE.md's Key
scripts section (npm run check row from #136, the playwright-admin.js
manual-only note from #139) -- kept both.
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