-
Notifications
You must be signed in to change notification settings - Fork 2
Production readiness improvements #223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Replace complex inline JSON config with simple CLI flags - Use --assert.assertions.*=warn flags to change error level to warning - This should resolve the workflow file parsing error
- Keep simplified Lighthouse CI command with warn flags - Include all necessary environment variables from remote branch - Resolve conflict between local and remote changes
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughReplaced manual Lighthouse CI CLI invocation with Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Runner as GitHub Actions Runner
participant CI as Workflow Job
participant LHCI as Lighthouse CI (autorun)
participant Preview as Preview App
participant Store as Artifact Storage
Runner->>CI: Trigger CI job
Note over CI,LHCI: Modified steps: use `lhci autorun` with warn assertions
CI->>LHCI: Invoke `lhci autorun` (env: `LHCI_GITHUB_APP_TOKEN`, `LHCI_TOKEN`)
LHCI->>Preview: Request target URL(s) and run audits
LHCI-->>CI: Return audit results & assertion statuses
CI->>Store: Upload performance/results artifacts (unchanged)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci-cd.yml (1)
483-487: Fix malformed FULL_HEALTH_URL query string (missing '?').Currently builds
/api/health&x-vercel..., which is invalid and may break protected-route bypass.Apply this diff:
- FULL_HEALTH_URL="$DEPLOYMENT_URL/api/health&x-vercel-set-bypass-cookie=true&x-vercel-protection-bypass=${{ secrets.VERCEL_BYPASS_TOKEN }}" + FULL_HEALTH_URL="$DEPLOYMENT_URL/api/health?x-vercel-set-bypass-cookie=true&x-vercel-protection-bypass=${{ secrets.VERCEL_BYPASS_TOKEN }}"
🧹 Nitpick comments (2)
.github/workflows/ci-cd.yml (2)
593-593: Prefer config file over long CLI flags; ensure target URLs are defined.The inline assertions are hard to maintain and easy to mistype. Move them into a
.lighthouserc.json(or.js) and point autorun to it. Also make surecollect.url(orserverCommand/staticDistDir) is configured solhciknows what to audit.Apply this diff here:
- lhci autorun --assert.assertions.categories:performance=warn --assert.assertions.categories:accessibility=warn --assert.assertions.categories:best-practices=warn --assert.assertions.categories:seo=warn --assert.assertions.first-contentful-paint=warn --assert.assertions.largest-contentful-paint=warn --assert.assertions.cumulative-layout-shift=warn --assert.assertions.total-blocking-time=warn --assert.assertions.speed-index=warn + lhci autorun --config=.lighthouserc.jsonExample config to add at repo root (adjust URLs/thresholds as needed):
{ "ci": { "collect": { "url": ["https://example.com/","https://example.com/page"], "numberOfRuns": 3 }, "assert": { "assertions": { "categories:performance": "warn", "categories:accessibility": "warn", "categories:best-practices": "warn", "categories:seo": "warn", "first-contentful-paint": "warn", "largest-contentful-paint": "warn", "cumulative-layout-shift": "warn", "total-blocking-time": "warn", "speed-index": "warn" } }, "upload": { "target": "filesystem", "outputDir": ".lighthouseci" } } }
592-593: Ensure Chrome is available for Lighthouse.Ubuntu runners may lack a usable Chrome/Chromium. Add a step to install/setup Chrome to avoid flaky failures.
Example step to add before running
lhci:- name: Setup Chrome uses: browser-actions/setup-chrome@v1To verify current availability, you can temporarily add:
google-chrome --version || chromium --version || echo "No Chrome found"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-cd.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Suite
🔇 Additional comments (1)
.github/workflows/ci-cd.yml (1)
592-593: Confirmed:@lhci/cli@0.12.xsupportslhci autorunwith--assert.assertions.*flags.
- Remove NEXT_PUBLIC_SUPABASE_URL, NEXT_PUBLIC_SUPABASE_ANON_KEY, and SUPABASE_SERVICE_ROLE_KEY - These environment variables are not used by lhci command - Follows principle of least privilege to prevent accidental exposure - Service role key should never be present in client contexts
Summary by CodeRabbit