Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions src/app/api/og/[username]/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ describe("OG Image Route", () => {
});

it("should generate image for valid username", async () => {
const mockFetch = vi.spyOn(global, "fetch").mockResolvedValue(
new Response(JSON.stringify({ name: "Valid User" }), { status: 200 })
const mockFetch = vi.spyOn(global, "fetch").mockImplementation(() =>
Promise.resolve(new Response(JSON.stringify({ name: "Valid User" }), { status: 200 }))
);
Comment on lines +36 to 38
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

According to the general rules, mock implementations that return a Promise should use async functions and have explicit return types to improve readability and maintain type safety.

Suggested change
const mockFetch = vi.spyOn(global, "fetch").mockImplementation(() =>
Promise.resolve(new Response(JSON.stringify({ name: "Valid User" }), { status: 200 }))
);
const mockFetch = vi.spyOn(global, "fetch").mockImplementation(async (): Promise<Response> =>
new Response(JSON.stringify({ name: "Valid User" }), { status: 200 })
);
References
  1. In TypeScript, ensure functions and mock implementations have explicit return types and use async functions for mocks returning Promises to maintain type safety and readability.


const req = new NextRequest("http://localhost/api/og/validuser");
const req = new NextRequest("http://localhost/api/og/validuser", { headers: { "x-forwarded-for": "test-ip-valid" } });
const res = await GET(req, { params: Promise.resolve({ username: "validuser" }) });

expect(res.status).toBe(200);
Expand All @@ -47,13 +47,14 @@ describe("OG Image Route", () => {
});

it("should return 429 and Retry-After header when rate limit is exceeded", async () => {
const mockFetch = vi.spyOn(global, "fetch").mockResolvedValue(
new Response(JSON.stringify({ name: "Valid User" }), { status: 200 })
const mockFetch = vi.spyOn(global, "fetch").mockImplementation(() =>
Promise.resolve(new Response(JSON.stringify({ name: "Valid User" }), { status: 200 }))
);
Comment on lines +50 to 52
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

According to the general rules, mock implementations that return a Promise should use async functions and have explicit return types to improve readability and maintain type safety.

Suggested change
const mockFetch = vi.spyOn(global, "fetch").mockImplementation(() =>
Promise.resolve(new Response(JSON.stringify({ name: "Valid User" }), { status: 200 }))
);
const mockFetch = vi.spyOn(global, "fetch").mockImplementation(async (): Promise<Response> =>
new Response(JSON.stringify({ name: "Valid User" }), { status: 200 })
);
References
  1. In TypeScript, ensure functions and mock implementations have explicit return types and use async functions for mocks returning Promises to maintain type safety and readability.


// Generate more than 50 requests to hit the rate limit (limit is 50 per minute)
// We must use a unique IP to not be affected by the rate limit of previous tests.
const req = new NextRequest("http://localhost/api/og/validuser", {
headers: { "x-forwarded-for": "test-ip" }
headers: { "x-forwarded-for": "test-ip-rate-limit" }
});

// Send 50 successful requests
Expand Down
253 changes: 138 additions & 115 deletions src/app/api/og/[username]/route.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,30 +11,7 @@
const ONE_DAY_IN_SECONDS = 24 * ONE_HOUR_IN_SECONDS;
const OG_CACHE_CONTROL = `public, max-age=${ONE_HOUR_IN_SECONDS}, s-maxage=${ONE_DAY_IN_SECONDS}, stale-while-revalidate=${ONE_DAY_IN_SECONDS}`;

export async function GET(
request: NextRequest,
{ params }: { params: Promise<{ username: string }> }
) {
const { username } = await params;

const forwarded = request.headers.get("x-forwarded-for");
const ip = forwarded ? forwarded.split(",").at(-1)?.trim() ?? "unknown" : "unknown";
const rateLimitResult = rateLimiter.check(ip);

if (!rateLimitResult.success) {
const retryAfterSec = Math.ceil((rateLimitResult.reset - Date.now()) / 1000);
return new Response("Rate limit exceeded", {
status: 429,
headers: { "Retry-After": String(retryAfterSec > 0 ? retryAfterSec : 0) },
});
}

if (!isValidGitHubUsername(username)) {
return new Response("Invalid username", { status: 400 });
}


// Fetch minimal profile data for the OG image
async function fetchGitHubProfile(username: string) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To adhere to the general rules for TypeScript, functions should have explicit return types to ensure type safety and API clarity. Defining an interface for the profile data also improves maintainability.

interface GitHubProfile {
  name: string;
  bio: string;
  avatarUrl: string;
  followers: number;
  publicRepos: number;
}

async function fetchGitHubProfile(username: string): Promise<GitHubProfile> {
References
  1. Maintain explicit return types for functions in TypeScript to ensure type safety and API clarity.

let name = username;
let bio = "";
let avatarUrl = "";
Expand All @@ -59,122 +36,168 @@
}
} catch (error) {
logger.error(`Failed to fetch GitHub profile for OG image: ${username}`, error);
// fallback to defaults
}

return new ImageResponse(
(
return { name, bio, avatarUrl, followers, publicRepos };
}

function OgImageTemplate({
username,
name,
bio,
avatarUrl,
followers,
publicRepos,
}: {
username: string;
name: string;
bio: string;
avatarUrl: string;
followers: number;
publicRepos: number;
}) {
Comment on lines +44 to +58
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To adhere to the general rules for TypeScript, functions (including React components) should have explicit return types.

Suggested change
function OgImageTemplate({
username,
name,
bio,
avatarUrl,
followers,
publicRepos,
}: {
username: string;
name: string;
bio: string;
avatarUrl: string;
followers: number;
publicRepos: number;
}) {
function OgImageTemplate({
username,
name,
bio,
avatarUrl,
followers,
publicRepos,
}: {
username: string;
name: string;
bio: string;
avatarUrl: string;
followers: number;
publicRepos: number;
}): JSX.Element {
References
  1. Maintain explicit return types for functions in TypeScript to ensure type safety and API clarity.

return (
<div
style={{
display: "flex",
flexDirection: "column",
width: "100%",
height: "100%",
backgroundColor: "#0d1117",
padding: "60px",
fontFamily: "sans-serif",
}}
>
{/* Top bar */}
<div
style={{
display: "flex",
flexDirection: "column",
width: "100%",
height: "100%",
backgroundColor: "#0d1117",
padding: "60px",
fontFamily: "sans-serif",
alignItems: "center",
marginBottom: "40px",
}}
>
{/* Top bar */}
<div
style={{
display: "flex",
alignItems: "center",
marginBottom: "40px",
}}
>
{avatarUrl && (
<img
src={sanitizeUrl(avatarUrl)}
alt=""
width={120}
height={120}
style={{
borderRadius: "60px",
marginRight: "32px",
border: "3px solid #30363d",
}}
/>
)}
<div style={{ display: "flex", flexDirection: "column" }}>
<div
style={{
fontSize: "48px",
fontWeight: 700,
color: "#e6edf3",
lineHeight: 1.2,
}}
>
{name}
</div>
<div
style={{
fontSize: "28px",
color: "#8b949e",
marginTop: "4px",
}}
>
@{username}
</div>
{avatarUrl && (
<img

Check warning on line 80 in src/app/api/og/[username]/route.tsx

View workflow job for this annotation

GitHub Actions / Lint

Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` or a custom image loader to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element
src={sanitizeUrl(avatarUrl)}
alt=""
width={120}
height={120}
style={{
borderRadius: "60px",
marginRight: "32px",
border: "3px solid #30363d",
}}
/>
)}
<div style={{ display: "flex", flexDirection: "column" }}>
<div
style={{
fontSize: "48px",
fontWeight: 700,
color: "#e6edf3",
lineHeight: 1.2,
}}
>
{name}
</div>
</div>

{/* Bio */}
{bio && (
<div
style={{
fontSize: "24px",
fontSize: "28px",
color: "#8b949e",
marginBottom: "40px",
lineHeight: 1.4,
overflow: "hidden",
textOverflow: "ellipsis",
maxHeight: "68px",
marginTop: "4px",
}}
>
{bio.length > 120 ? `${bio.slice(0, 120)}…` : bio}
@{username}
</div>
)}
</div>
</div>

{/* Stats */}
{/* Bio */}
{bio && (
<div
style={{
display: "flex",
gap: "48px",
marginTop: "auto",
fontSize: "24px",
color: "#8b949e",
marginBottom: "40px",
lineHeight: 1.4,
overflow: "hidden",
textOverflow: "ellipsis",
maxHeight: "68px",
}}
>
<div style={{ display: "flex", flexDirection: "column" }}>
<div style={{ fontSize: "36px", fontWeight: 700, color: "#58a6ff" }}>
{publicRepos.toLocaleString()}
</div>
<div style={{ fontSize: "18px", color: "#8b949e", marginTop: "4px" }}>
Repositories
</div>
{bio.length > 120 ? `${bio.slice(0, 120)}…` : bio}
</div>
)}

{/* Stats */}
<div
style={{
display: "flex",
gap: "48px",
marginTop: "auto",
}}
>
<div style={{ display: "flex", flexDirection: "column" }}>
<div style={{ fontSize: "36px", fontWeight: 700, color: "#58a6ff" }}>
{publicRepos.toLocaleString()}
</div>
<div style={{ display: "flex", flexDirection: "column" }}>
<div style={{ fontSize: "36px", fontWeight: 700, color: "#58a6ff" }}>
{followers.toLocaleString()}
</div>
<div style={{ fontSize: "18px", color: "#8b949e", marginTop: "4px" }}>
Followers
</div>
<div style={{ fontSize: "18px", color: "#8b949e", marginTop: "4px" }}>
Repositories
</div>
</div>

{/* Branding */}
<div
style={{
display: "flex",
justifyContent: "flex-end",
marginTop: "32px",
}}
>
<div style={{ fontSize: "20px", color: "#484f58" }}>
GitHub User Summary
<div style={{ display: "flex", flexDirection: "column" }}>
<div style={{ fontSize: "36px", fontWeight: 700, color: "#58a6ff" }}>
{followers.toLocaleString()}
</div>
<div style={{ fontSize: "18px", color: "#8b949e", marginTop: "4px" }}>
Followers
</div>
</div>
</div>

{/* Branding */}
<div
style={{
display: "flex",
justifyContent: "flex-end",
marginTop: "32px",
}}
>
<div style={{ fontSize: "20px", color: "#484f58" }}>
GitHub User Summary
</div>
</div>
</div>
);
}

export async function GET(
request: NextRequest,
{ params }: { params: Promise<{ username: string }> }
) {
Comment on lines +174 to +177
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To adhere to the general rules for TypeScript, the GET handler should have an explicit return type.

Suggested change
export async function GET(
request: NextRequest,
{ params }: { params: Promise<{ username: string }> }
) {
export async function GET(
request: NextRequest,
{ params }: { params: Promise<{ username: string }> }
): Promise<Response> {
References
  1. Maintain explicit return types for functions in TypeScript to ensure type safety and API clarity.

const { username } = await params;

const forwarded = request.headers.get("x-forwarded-for");
const ip = forwarded ? forwarded.split(",").at(-1)?.trim() ?? "unknown" : "unknown";
const rateLimitResult = rateLimiter.check(ip);

if (!rateLimitResult.success) {
const retryAfterSec = Math.ceil((rateLimitResult.reset - Date.now()) / 1000);
return new Response("Rate limit exceeded", {
status: 429,
headers: { "Retry-After": String(retryAfterSec > 0 ? retryAfterSec : 0) },
});
}

if (!isValidGitHubUsername(username)) {
return new Response("Invalid username", { status: 400 });
}

const profileData = await fetchGitHubProfile(username);

return new ImageResponse(
(
<OgImageTemplate username={username} {...profileData} />
),
{
width: 1200,
Expand Down
Loading