Skip to content

fix: improve error message for invalid icon imports#4062

Open
FadhlanR wants to merge 1 commit intomainfrom
cs-10201-bad-import-will-produce-an-unhelpful-message
Open

fix: improve error message for invalid icon imports#4062
FadhlanR wants to merge 1 commit intomainfrom
cs-10201-bad-import-will-produce-an-unhelpful-message

Conversation

@FadhlanR
Copy link
Contributor

@FadhlanR FadhlanR commented Feb 25, 2026

Problem

When a card definition has a static icon that is a bad named import (e.g. import { NonExistent } from '@cardstack/boxel-icons/non-existing-path/nope'), JavaScript silently resolves it to undefined. This caused the prerenderer to produce a cryptic [data-prerender] has no child element to capture error, giving developers or ai-bot no actionable information.

Fix

Added a guard in the render/icon route's model() hook that checks whether cardTypeIcon() resolved to a valid component. If not, it throws a descriptive error: static icon is undefined — check that the import resolves to a valid icon component. Because Ember is still intact at this point, the error propagates via the parent render route's error action hook into the render.error route, which renders a [data-prerender-error] element. The prerenderer's captureResult picks this up and surfaces the message in the error document.

Before
Screenshot 2026-02-26 at 13 15 32

After
Screenshot 2026-02-26 at 13 06 31

@github-actions
Copy link

Preview deployments

@github-actions
Copy link

Host Test Results

    1 files  ±0      1 suites  ±0   1h 35m 26s ⏱️ - 3m 57s
1 878 tests ±0  1 863 ✅ ±0  15 💤 ±0  0 ❌ ±0 
1 893 runs  ±0  1 878 ✅ ±0  15 💤 ±0  0 ❌ ±0 

Results for commit 0827549. ± Comparison against base commit c2e3fb9.

@FadhlanR FadhlanR marked this pull request as ready for review February 26, 2026 06:16
@FadhlanR FadhlanR requested review from a team and habdelra February 26, 2026 06:16
Copy link
Contributor

@backspace backspace left a comment

Choose a reason for hiding this comment

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

Is it possible to show the actual incorrect import? But regardless, this is a huge DX improvement 🎉

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request improves error messages when card definitions have invalid or missing static icon imports. Previously, when a card's static icon property resolved to undefined (e.g., from a bad import), the prerenderer produced a cryptic error about missing child elements. This PR adds validation in the icon route to detect this case early and provide a clear, actionable error message.

Changes:

  • Added guard in icon route's model hook to validate icon component exists before returning
  • Added test fixture and test case for bad icon import scenario

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/host/app/routes/render/icon.ts Added validation to check if cardTypeIcon returns undefined and throw descriptive error
packages/realm-server/tests/prerendering-test.ts Added test fixture with undefined icon and test case verifying error message

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

let component = cardTypeIcon(instance);
if (!component) {
throw new Error(
`static icon is undefined — check that the import resolves to a valid icon component`,
Copy link
Contributor

Choose a reason for hiding this comment

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

lets improve this message by including in the error the instance's constructor name. that will help who ever read the error to know which card def has the bad icon definition

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.

4 participants