Skip to content

Stop unknown plugin icon names from 500ing every page - #486

Draft
simonhamp wants to merge 2 commits into
mainfrom
fix-heroicon-dynamic-component
Draft

Stop unknown plugin icon names from 500ing every page#486
simonhamp wants to merge 2 commits into
mainfrom
fix-heroicon-dynamic-component

Conversation

@simonhamp

Copy link
Copy Markdown
Member

Fixes Nightwatch issue #53InvalidArgumentException: Unable to locate a class or view for component [heroicon-o-image].

Root cause

plugins.icon_name is free text typed by developers — the "Heroicon name" input on the plugin Icon card. Show::updateIcon() only validated its shape (/^[a-z0-9-]+$/), never whether the icon actually exists. Views then rendered it as:

<x-dynamic-component :component="'heroicon-o-' . $plugin->icon_name" />

Blade resolves that alias at component-resolution time, so an unknown name throws InvalidArgumentException and 500s the entire page. The two names in the reports simply aren't Heroicons — it's photo, not image, and map-pin, not location. Once saved, a single bad row poisons every page that renders that plugin.

The blast radius was wider than the reported route: 10 render sites across 7 files, including the public plugin directory card, the public plugin listing, the cart, the Ultra index, the team page, and purchased plugins.

Changes

  1. Render defensivelyPlugin::getIconComponent() resolves the stored name through the blade-icons factory and falls back to heroicon-o-cube when it doesn't exist. All 10 call sites now use it. This is what recovers the rows already broken in production.
  2. Stop the bad data at the doorShow::updateIcon() now rejects names that aren't real Heroicons, with a message pointing at heroicons.com.
  3. Copy fix — the input hint read "e.g., cube, sparkles, bolt", which invites guessing. It now says to use the name exactly as it appears on heroicons.com, and includes photo/map-pin as examples.

The shape regex runs before the factory lookup, which also blocks the str_replace('.', '/') path traversal in blade-icons' file resolution (covered by a test).

Verification

New tests/Feature/PluginIconFallbackTest.php covers name validation, the render fallback, the four affected page types, and both validation branches on save.

I reverted the view changes and confirmed the tests reproduce the reported error verbatim — Unable to locate a class or view for component [heroicon-o-image]. — then restored and confirmed green. 82 tests across the plugin/cart/ultra/team suites pass. Pint clean.

Notes for review

  • Existing bad rows are deliberately left as-is. They now render a cube instead of 500ing, and the owner gets a clear error next time they save the icon. Nulling them out in a migration would silently discard developer intent — happy to add one if you'd prefer.
  • A longer-term option worth considering: replace the free-text input with a picker, so an invalid name is unrepresentable rather than merely rejected.
  • Unrelated: php artisan test with a broad --filter OOMs at the default 128M CLI memory limit (dies in a Filament GridDirection.php). Not touched here.

🤖 Generated with Claude Code

simonhamp and others added 2 commits August 25, 2026 14:57
plugins.icon_name is free text typed by developers, but was only validated
for shape, never for whether the Heroicon actually exists. Views rendered it
via <x-dynamic-component :component="'heroicon-o-' . $plugin->icon_name" />,
which throws InvalidArgumentException at component resolution when the name
is unknown — taking down the whole page. "image" (should be "photo") and
"location" (should be "map-pin") were both saved in production.

Add Plugin::getIconComponent(), which resolves the name through the
blade-icons factory and falls back to heroicon-o-cube, and use it at all ten
render sites (plugin card, public listing, cart, Ultra index, team page,
purchased plugins, developer plugin page). This recovers the rows already
broken in production.

Also reject non-existent icon names in Show::updateIcon() so no new bad data
lands, and point the input hint at heroicons.com instead of inviting guesses.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The messages() relation already sorts with oldest(), so the test's
latest('id') landed as a secondary key: `order by created_at asc, id desc`.
That returns the *earliest* message, not the newest. Laravel stores
timestamps at second precision, so the test only passed when the admin
message and the developer reply happened to land in the same second — it
went red on CI the moment they straddled a second boundary.

Order the relation by id, which is monotonic and unique, so the same
chronological order holds with ties defined. Clear the inherited sort in the
test with reorder(), and travel a second before replying so the cross-second
case is always exercised rather than hidden by equal timestamps.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant