Add Lighthouse CI for automated accessibility validation#19
Conversation
…ror and output it
There was a problem hiding this comment.
Pull request overview
This PR adds Lighthouse CI for automated accessibility, performance, SEO, and best-practices validation on every pull request. It includes metadata validation scripts, improved component accessibility, comprehensive test coverage, and a split CI workflow for faster feedback.
Key Changes:
- Adds Lighthouse CI with 100% thresholds for accessibility/SEO/best-practices (95% for performance)
- Implements metadata validation for OpenGraph and Twitter Card tags across all pages
- Makes blog post descriptions required for better SEO and social sharing
- Splits CI into fast code quality checks (~1min) vs slower build validation (~3-5min)
Reviewed changes
Copilot reviewed 29 out of 31 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| validation/validate-metadata.ts | Validates OpenGraph/Twitter metadata in built HTML with comprehensive error checking |
| validation/validate-metadata.test.ts | Unit tests for metadata validation pure functions |
| validation/parse-lighthouse-errors.ts | Custom parser to extract greppable attributes from Lighthouse failures for CI debugging |
| validation/parse-lighthouse-errors.test.ts | Unit tests for Lighthouse error parsing and formatting |
| validation/config.ts | Shared constants for validation scripts |
| ui/link.tsx | Forwards ariaLabel prop to enable accessible icon-only links |
| ui/link.test.tsx | Comprehensive tests for Link component accessibility features |
| ui/icon.tsx | Adds empty alt attribute to mark icons as decorative |
| ui/icon.test.tsx | Tests for Icon component decorative behavior |
| ui/footer.tsx | Updates to use ariaLabel prop for social links |
| ui/footer.test.tsx | Tests for Footer social link accessibility |
| package.json | Adds Lighthouse CI dependency and script |
| lighthouserc.cjs | Lighthouse CI configuration with SEO exemption for 404 pages |
| io/notion/schemas/post.ts | Makes description field required for SEO |
| io/notion/getPosts.ts | Removes redundant Title/Slug filters (validated by schema) |
| .github/workflows/checks.yml | New fast CI workflow for format/lint/typecheck/test |
| .github/workflows/build-validation.yml | Restructured workflow for build, metadata validation, and Lighthouse |
| app/(prose)/[slug]/page.tsx | Uses required description field (removes null coalescing) |
| app/rss.xml/route.ts | Uses required description field (removes null coalescing) |
| *.test.tsx | Updates test fixtures to include required description field |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,333 @@ | |||
| #!/usr/bin/env tsx | |||
There was a problem hiding this comment.
The shebang uses 'tsx' directly, which requires tsx to be globally installed. Consider using '#!/usr/bin/env node' with tsx invocation in package.json scripts instead, or document that tsx must be installed globally. This makes the script more portable across different development environments.
| #!/usr/bin/env tsx | |
| #!/usr/bin/env node |
| { property: 'Destination', multi_select: { contains: 'blog' } }, | ||
| { property: 'Status', status: { equals: 'Published' } }, // redundant if "First published" also used? | ||
| { property: 'Title', title: { is_not_empty: true } }, | ||
| { property: 'Slug', rich_text: { is_not_empty: true } }, | ||
| // NOTE: link posts don't have featured images atm | ||
| // { property: 'Featured image', url: { is_not_empty: true } }, | ||
| { property: 'First published', date: { on_or_before: new Date().toISOString() } }, |
There was a problem hiding this comment.
The removed Title and Slug filters had a comment explaining they were redundant (now validated by schema). Consider adding a brief comment explaining that Title, Slug, and Description validation now happens at the schema level (PostListItemSchema) rather than in the Notion query, to clarify why these filters were removed.
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '20' | ||
| node-version-file: '.nvmrc' |
There was a problem hiding this comment.
The workflow references '.nvmrc' for node version, but this file is not included in the diff. Ensure the .nvmrc file exists in the repository and contains a valid Node.js version. If it doesn't exist, this workflow step will fail.
PR Review: Add Lighthouse CI for automated accessibility validationOverall AssessmentThis is an excellent PR that significantly improves the project's quality assurance infrastructure. The implementation is thorough, well-tested, and follows best practices. The code quality is high with strong separation of concerns, comprehensive test coverage, and excellent documentation. Recommendation: Approve ✅ Strengths1. Architecture & Design ⭐
2. Code Quality ⭐
3. Test Coverage ⭐
4. Developer Experience ⭐
Issues & RecommendationsCritical IssuesNone found. Minor Issues1. Node Version Consistency (Low Priority)File: The workflow uses 2. Potential Race Condition in Lighthouse Cache (Very Low Priority)File: Chrome cache is keyed on key: lighthouse-chrome-${{ runner.os }}-${{ hashFiles('package-lock.json') }}-${{ hashFiles('node_modules/@lhci/cli/package.json') }}3. Hardcoded Site URL in TestsFile: Tests hardcode import { SITE_URL } from '@/utils/metadata'
expect(getExpectedCanonicalUrl('index.html')).toBe(SITE_URL)4. Missing Test for Image Fetch TimeoutFile: The Performance ConsiderationsPositive
Optimization OpportunityFile: The Security ConsiderationsPositive
No Issues FoundThe validation scripts only read build artifacts and don't execute user input. Security posture is strong. Accessibility Impact ⭐This PR delivers significant a11y improvements:
The automated validation will catch issues that manual testing often misses. Suggested Follow-ups (Not Blockers)From the PR description and
Final NotesThis PR demonstrates excellent software engineering practices:
The commit history shows thoughtful iteration and refinement (32 commits evolving the implementation). The final result is production-ready. Great work! 🎉 |
Replace Next.js Image component with native img element to have full control over srcset generation. Remove hardcoded dpr_2.0 from Cloudinary transformations - DPR is now handled via srcset (192w, 384w, 576w). Also increase staticPageGenerationTimeout to 180s for pages with slow external API calls (TMDB, iTunes, Notion). Results: - Lighthouse image-responsive audit: 100% (was 50% with 19 failures) - Images load at appropriate sizes for device DPR - Build no longer times out on /likes page
Update test expectations to match new implementation that removed hardcoded dpr_2.0 parameter. DPR is now handled via srcset in the component, not in the transform function.
✅ What
noindex)LinkforwardsariaLabelto DOM,Iconmarked as decorativeLink,Icon, andFootercomponents🤔 Why
👩🔬 How to validate
npm run test:cinpm run typechecknpm run buildnpm run test:metadatanpm run lighthouse