fix: improve avatar rendering - #9048
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview In
Reviewed by Cursor Bugbot for commit 11545db. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9048 +/- ##
==========================================
+ Coverage 75.12% 75.13% +0.01%
==========================================
Files 102 102
Lines 9109 9121 +12
Branches 372 376 +4
==========================================
+ Hits 6843 6853 +10
- Misses 2262 2264 +2
Partials 4 4 ☔ View full report in Codecov by Harness. |
bmuenzenmeyer
left a comment
bmuenzenmeyer
left a comment
There was a problem hiding this comment.
self-review
| "id": "tracy-hinds", | ||
| "name": "Tracy Hinds" | ||
| "id": 1981088, | ||
| "name": "Tracy Hinds", | ||
| "website": "https://github.com/hackygolucky" |
There was a problem hiding this comment.
example of one of the broken username re-names
| "webiste": "https://github.com/sam-github" | ||
| "website": "https://github.com/sam-github" |
There was a problem hiding this comment.
example of one of the typos
| "id": "mike-dolan", | ||
| "name": "Mike Dolan" | ||
| "id": 7398917, | ||
| "name": "Mike Dolan", | ||
| "website": "https://github.com/mkdolan" |
There was a problem hiding this comment.
the other rename this resolves
| "id": "nodejs", | ||
| "id": 9950313, | ||
| "name": "Node.js Release Working Group", | ||
| "website": "https://github.com/nodejs/release" | ||
| }, | ||
| "Node.js Technical Steering Committee": { | ||
| "id": "nodejs", | ||
| "id": 9950313, |
There was a problem hiding this comment.
note the same avatar will resolve, but proper bylines
|
|
||
| // Extracts the GitHub username from an author's profile URL, since the numeric | ||
| // `id` is only used to build a stable avatar URL and no longer holds the handle | ||
| const getGitHubUsername = (website?: string) => website?.split('/').pop(); |
There was a problem hiding this comment.
this is slightly un-ideal but works
There was a problem hiding this comment.
What about the inverse? e.g. instead of supplying website, we supply username, and this builds the website
There was a problem hiding this comment.
During lint, we can check if username === id?
There was a problem hiding this comment.
Pull request overview
Improves how author avatars are rendered across the site by switching from username-based avatar URLs to GitHub’s stable numeric id, while keeping existing fallbacks for unknown authors and updating the author data accordingly.
Changes:
- Update
getGitHubAvatarUrlto support stable id-based avatar URLs (with an opt-in legacy username-based path). - Adjust author mapping logic to look up authors via their GitHub URL “handle/slug” and use numeric ids for avatar rendering.
- Update tests, author types, and
authors.jsonentries (including fixingwebiste→websitetypos) to match the new model.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/site/util/github.ts | Switch avatar URL generation to use id-based /u/:id by default, with a legacy option. |
| apps/site/util/author.ts | Update author lookup/nickname logic to use URL-derived slugs and numeric ids for avatars. |
| apps/site/util/tests/github.test.mjs | Refresh and expand tests for the new avatar URL behavior (id vs legacy). |
| apps/site/util/tests/author.test.mjs | Update author mapping tests to validate id-based avatar URLs and distinct bylines. |
| apps/site/types/author.ts | Change Author.id type from string to number to reflect GitHub numeric ids. |
| apps/site/authors.json | Replace username “id” values with numeric GitHub ids and fix website key typos. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📦 Build Size ComparisonSummary
Changes🔄 Modified Routes (4)
|
|
|
||
| // Extracts the GitHub username from an author's profile URL, since the numeric | ||
| // `id` is only used to build a stable avatar URL and no longer holds the handle | ||
| const getGitHubUsername = (website?: string) => website?.split('/').pop(); |
There was a problem hiding this comment.
During lint, we can check if username === id?
Description
Improves avatar rendering by using the durable, immutable GH
idand the currently observed url from GH. Notably, this works for both users and orgs, the inciting ask from https://openjs-foundation.slack.com/archives/CVAMEJ4UV/p1785347765682829Turns out, the old avatars.githubusercontent url was no longer observed in GH API responses. This new one is represented there, on their website, and works for both users and orgs (with the caveat that we need to use an id).
IDs are better anyways, as evident by two of our
authors.jsonvalues changing out from under us. I updated both of those entries while here, as well as fixing some typos in the file that were breaking the rendering of those authors.Validation
This should NOT affect author or blog post workflows, to my knowledge, as no logic was driven off of the username-as-id. Meaning, no blog posts need to know the author - they keep using the full names.
Related Issues
Check List
pnpm formatto ensure the code follows the style guide.pnpm testto check if all tests are passing.pnpm buildto check if the website builds without errors.