Conversation
…rom campaign banner The campaign banner (decentraland-ui2 Banner) lazily imports @contentful/rich-text-react-renderer, an optional peer dependency that was never installed. Vite silently bundled the unresolved import as an empty stub module, so rendering any banner with rich text crashed with "documentToReactComponents is not a function" and, with no error boundary above it, blanked the whole app (currently reproducible on decentraland.zone/builder). Install the package so the import resolves to the real renderer, and wrap the banner usages in an error boundary so a broken banner degrades to no banner instead of taking down the page. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tall-contentful-rich-text-renderer
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
decentraland-bot
left a comment
There was a problem hiding this comment.
Review Summary
Verdict: ✅ Approve
Clean, well-scoped fix for a production-impacting bug (blank page when a Contentful campaign banner with rich text is active).
Changes Reviewed
| File | Assessment |
|---|---|
package.json |
✅ Correctly adds @contentful/rich-text-react-renderer as a production dependency — this was a missing peer dep of decentraland-ui2 that Vite silently stubbed as an empty module |
src/components/CampaignBanner/ (3 files) |
✅ Clean error boundary wrapper following project conventions (component + types + barrel export) |
src/components/HomePage/HomePage.tsx |
✅ Banner replaced with CampaignBanner |
src/components/CollectionsPage/CollectionsPage.tsx |
✅ Banner replaced with CampaignBanner |
package-lock.json |
✅ Lock file consistent with dependency addition |
Analysis
Root cause fix — Installing the missing @contentful/rich-text-react-renderer peer dependency resolves the documentToReactComponents is not a function TypeError that was crashing the page.
Defense-in-depth — The CampaignBanner error boundary is a good addition: if a campaign banner ever fails to render for any reason, the page degrades gracefully (banner disappears) instead of going blank. Class component usage is correct — React requires class components for error boundaries.
Architecture — Error boundary is placed at the right granularity. A single reusable wrapper (CampaignBanner) avoids duplicating boundary logic at each of the two usage sites. Both existing Banner usage sites in the codebase are covered.
Component structure — The 3-file pattern (.tsx + .types.ts + index.ts) matches the project's established component conventions.
Security
No security issues found. No hardcoded secrets, no XSS surface (only a string id prop is passed), no dangerouslySetInnerHTML, and @contentful/rich-text-react-renderer is a well-maintained package with no known vulnerabilities.
CI Status
Audit, test, and Vercel checks are still running at time of review.
Findings
No P0 or P1 issues. No P2 findings worth flagging — the implementation is minimal and correct.
Reviewed by Jarvis 🤖 · Requested by RocioCM via GitHub
…tall-contentful-rich-text-renderer
Coverage Report for CI Build 32740931716Coverage remained the same at 53.595%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats💛 - Coveralls |
What
Why
The campaign banner lazily loads
@contentful/rich-text-react-renderer, an optional peer dependency of decentraland-ui2 that was never installed. Vite silently bundled the unresolved import as an empty stub module, so rendering any banner with rich text threwdocumentToReactComponents is not a functionand, with no error boundary above it, blanked the entire app.How to test
master: the page is blank with theTypeErrorabove in the console.npm ci && npx vite build,dist/assets/rich-text-react-renderer*.jsis the real ~11KB module on this branch versus a 33-byte empty stub onmaster.