Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a client-side Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/site/src/app/newsletter/newsletter-signup.tsx`:
- Around line 66-69: The statusMessage paragraph is not in an ARIA live region
so screen-readers may not announce updates; update the JSX that renders
statusMessage (the <p> that references statusMessage.text) to be an ARIA live
region (e.g., add role="status" and aria-live="polite" and aria-atomic="true")
and ensure the element remains in the DOM even when statusMessage is empty
(render an empty/visually-hidden element or text node) so updates to the
statusMessage variable are announced to assistive tech.
- Around line 58-64: Replace the nested <input> inside the Button component with
a native button submit by passing type="submit" and the label text as the
Button's children (use the existing isSubmitting conditional to show "..." vs
"Sign me up") so the Button's disabled prop correctly controls the element;
update the Button invocation (the component named Button in this file) to
include type="submit" and remove the inner input. Also make the status message
element (the success/error message rendered after the form) accessible by adding
either aria-live="polite" or role="status" so screen readers announce changes.
In `@apps/site/src/app/newsletter/page.tsx`:
- Around line 27-30: The fetch call that reads the RSS feed (const res = await
fetch(...)) currently parses the response body even for non-2xx responses;
update the code in page.tsx to check res.ok after the fetch and before calling
res.text() (and do the same for the second fetch at the other occurrence), and
if !res.ok log an error including res.status and either throw or return the
fallback path so the parser isn't given partial/HTML bodies; reference the
existing variables res and xml and ensure the fallback/logging is explicit so
failures are visible.
- Around line 78-79: The top-level await in NewsletterPage blocks rendering;
move the call to getLatestBlogPosts into a Suspense-wrapped async child so the
signup form renders immediately. Create a new async component (e.g.,
LatestBlogPosts) that calls getLatestBlogPosts(3) and returns the blog cards,
then replace the direct await in NewsletterPage with <Suspense fallback={/*
lightweight placeholder or spinner */}><LatestBlogPosts /></Suspense>. Keep
NewsletterPage synchronous and ensure the Suspense fallback matches the layout
to avoid layout shift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 01c35fa4-e699-4efd-9312-bfb7d189cd75
📒 Files selected for processing (2)
apps/site/src/app/newsletter/newsletter-signup.tsxapps/site/src/app/newsletter/page.tsx
aidankmcalister
force-pushed
the
feat/newsletter-page
branch
from
c0e61fa to
27e036c
Compare
aidankmcalister
force-pushed
the
feat/newsletter-page
branch
from
27e036c to
83ba77d
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/site/src/app/newsletter/page.tsx (1)
117-117: Considertarget="_blank"for external blog links if they're full URLs.If
post.linkcontains absolute URLs (e.g.,https://www.prisma.io/blog/...), the current implementation keeps users in the same tab. This is fine if that's intentional since it's the same domain family, but worth confirming the expected behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/newsletter/page.tsx` at line 117, If post.link can be an absolute external URL, update the rendering around the Link element (the Link with key={post.link} and className="group") to open externals in a new tab by adding target="_blank" and rel="noopener noreferrer" for security; implement this conditionally by detecting absolute URLs (e.g., post.link startsWith('http') or using URL parsing) so internal/local links remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/site/src/app/newsletter/page.tsx`:
- Line 117: If post.link can be an absolute external URL, update the rendering
around the Link element (the Link with key={post.link} and className="group") to
open externals in a new tab by adding target="_blank" and rel="noopener
noreferrer" for security; implement this conditionally by detecting absolute
URLs (e.g., post.link startsWith('http') or using URL parsing) so internal/local
links remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9c104b88-41e8-45fb-b423-48549c705c4a
📒 Files selected for processing (2)
apps/site/src/app/newsletter/newsletter-signup.tsxapps/site/src/app/newsletter/page.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/site/src/app/newsletter/newsletter-signup.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/site/src/app/newsletter/newsletter-signup.tsx (2)
62-68:⚠️ Potential issue | 🟠 MajorUse
Buttonas the submit control directly.Line 62 currently wraps an interactive
<input type="submit">(Lines 63-67) insideButton, which creates nested interactive elements and invalid HTML semantics.Suggested fix
- <Button variant="ppg" size="2xl" disabled={disabled}> - <input - type="submit" - value={isSubmitting ? "..." : "Sign me up"} - className="cursor-pointer" - /> - </Button> + <Button type="submit" variant="ppg" size="2xl" disabled={disabled}> + {isSubmitting ? "..." : "Sign me up"} + </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/newsletter/newsletter-signup.tsx` around lines 62 - 68, Replace the nested <input type="submit"> with the Button component as the form submit control: remove the inner input and render Button with type="submit", keep disabled={disabled}, preserve variant="ppg" and size="2xl", and use the isSubmitting ? "..." : "Sign me up" string as the Button label so there are no nested interactive elements (look for the Button usage around the newsletter signup form and the isSubmitting/disabled variables).
70-74:⚠️ Potential issue | 🟠 MajorAnnounce submit status changes via a live region.
Lines 70-74 render important success/error feedback, but without live-region semantics this update may not be announced to screen-reader users.
Suggested fix
- {statusMessage ? ( - <p className={`m-0 text-sm ${statusMessage.className}`}> - {statusMessage.text} - </p> - ) : null} + <p + role={error ? "alert" : "status"} + aria-live={error ? "assertive" : "polite"} + aria-atomic="true" + className={`m-0 text-sm ${statusMessage?.className ?? ""}`} + > + {statusMessage?.text ?? ""} + </p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/newsletter/newsletter-signup.tsx` around lines 70 - 74, The statusMessage paragraph isn't exposed to assistive tech; wrap or replace the conditional rendering so the status text is placed inside an ARIA live region (e.g., add role="status" and aria-live="polite" and aria-atomic="true") so screen readers announce updates from the newsletter-signup component; update the JSX that renders statusMessage (the element using statusMessage.text / statusMessage.className) to include these attributes or render the text inside a dedicated <div> with those attributes.apps/site/src/app/newsletter/page.tsx (1)
26-30:⚠️ Potential issue | 🟡 MinorHandle non-2xx RSS responses before parsing, and log failures.
Line 29 parses response text even when the RSS endpoint returns an error status, and Lines 60-62 swallow parse/fetch errors without telemetry.
Suggested fix
const res = await fetch("https://www.prisma.io/blog/rss.xml", { next: { revalidate: 3600 }, }); + if (!res.ok) { + console.error("Failed to fetch blog RSS", res.status, res.statusText); + return []; + } const xml = await res.text(); @@ - } catch { + } catch (error) { + console.error("Failed to parse blog RSS", error); return []; }Also applies to: 60-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/newsletter/page.tsx` around lines 26 - 30, The fetch of the RSS feed currently calls res.text() and proceeds to parsing even when the HTTP response is non-2xx and later parse/fetch errors are swallowed; update the fetch logic around the variables res and xml in page.tsx to check res.ok (or status) immediately after the fetch and handle non-2xx by logging the status and statusText (using the same logger/telemetry the app uses) and return or throw a descriptive error before calling res.text(); also ensure the try/catch around the parsing (the block that currently swallows errors at lines ~60-62) logs the exception to telemetry and rethrows or returns a safe fallback so failures are not silently ignored (reference res, xml, and the parse block where errors are currently caught).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/site/src/app/newsletter/page.tsx`:
- Line 10: Replace the typo "Get releases updates" with "Get release updates"
everywhere it appears in this file: search for the exact string "Get releases
updates, tutorials, and more content delivered to your inbox monthly." (and the
duplicate occurrences around lines ~99-100) and update both the metadata export
and any JSX/body text where that string is used so the user-facing copy reads
"Get release updates, tutorials, and more content delivered to your inbox
monthly."
---
Duplicate comments:
In `@apps/site/src/app/newsletter/newsletter-signup.tsx`:
- Around line 62-68: Replace the nested <input type="submit"> with the Button
component as the form submit control: remove the inner input and render Button
with type="submit", keep disabled={disabled}, preserve variant="ppg" and
size="2xl", and use the isSubmitting ? "..." : "Sign me up" string as the Button
label so there are no nested interactive elements (look for the Button usage
around the newsletter signup form and the isSubmitting/disabled variables).
- Around line 70-74: The statusMessage paragraph isn't exposed to assistive
tech; wrap or replace the conditional rendering so the status text is placed
inside an ARIA live region (e.g., add role="status" and aria-live="polite" and
aria-atomic="true") so screen readers announce updates from the
newsletter-signup component; update the JSX that renders statusMessage (the
element using statusMessage.text / statusMessage.className) to include these
attributes or render the text inside a dedicated <div> with those attributes.
In `@apps/site/src/app/newsletter/page.tsx`:
- Around line 26-30: The fetch of the RSS feed currently calls res.text() and
proceeds to parsing even when the HTTP response is non-2xx and later parse/fetch
errors are swallowed; update the fetch logic around the variables res and xml in
page.tsx to check res.ok (or status) immediately after the fetch and handle
non-2xx by logging the status and statusText (using the same logger/telemetry
the app uses) and return or throw a descriptive error before calling res.text();
also ensure the try/catch around the parsing (the block that currently swallows
errors at lines ~60-62) logs the exception to telemetry and rethrows or returns
a safe fallback so failures are not silently ignored (reference res, xml, and
the parse block where errors are currently caught).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ba073e1e-58e5-495a-9d43-62c223817193
📒 Files selected for processing (2)
apps/site/src/app/newsletter/newsletter-signup.tsxapps/site/src/app/newsletter/page.tsx
mhartington
approved these changes
Mar 27, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/blog/src/lib/rss.ts`:
- Around line 28-33: The image URL logic discards already-absolute cover URLs
because it only handles rel starting with "/" — change the conditional in the
image resolution code (variables cover, rel, image and helper
withBlogBasePathForImageSrc) so that: if rel starts with "/" then prefix baseUrl
(baseUrl + rel), else if rel already is an absolute URL (starts with "http://",
"https://" or protocol-relative "//") keep rel as-is, otherwise fall back to
undefined; update the expression that computes image to implement this three-way
check so absolute URLs are preserved.
In `@apps/site/next.config.mjs`:
- Around line 53-54: The CSP currently includes loopback image origins
("http://localhost:3002" and "http://127.0.0.1:3002") unconditionally; update
the headers generation (the exported headers function / contentSecurityPolicy
variable in next.config.mjs) to append those two origins to the img-src only in
development (e.g. when process.env.NODE_ENV === 'development' or a DEV flag),
and ensure the production branch/build path constructs the img-src without those
loopback origins so deployed pages do not permit localhost or 127.0.0.1.
In `@apps/site/src/app/newsletter/page.tsx`:
- Around line 32-35: The fetch in page.tsx (used by NewsletterPage) hardcodes
"http://localhost:3002/blog/rss.xml" which breaks in preview/prod; change it to
resolve the feed URL from the deployment/config instead of localhost — e.g.
derive the origin from a runtime-config/env var (NEXT_PUBLIC_SITE_URL or
NEXT_PUBLIC_FEED_URL) or build the URL with new URL('/blog/rss.xml',
process.env.NEXT_PUBLIC_SITE_URL) and use that in the fetch call (keep next: {
revalidate: 3600 } intact); update any server-component code that calls fetch to
use this resolved URL so the feed works in preview/production.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 801d789a-70af-49d4-b5cf-198360ee1d9a
📒 Files selected for processing (4)
apps/blog/src/lib/rss.tsapps/site/next.config.mjsapps/site/src/app/newsletter/page.tsxpackages/ui/src/lib/rss.ts
Comment on lines
+28
to
+33
| const cover = page.data.metaImagePath ?? page.data.heroImagePath; | ||
| const rel = cover ? withBlogBasePathForImageSrc(cover) : ""; | ||
| const image = | ||
| rel.startsWith("/") | ||
| ? `${baseUrl.replace(/\/$/, "")}${rel}` | ||
| : undefined; |
Contributor
There was a problem hiding this comment.
Preserve already-absolute cover URLs.
withBlogBasePathForImageSrc() leaves non-root URLs unchanged. The current rel.startsWith("/") check turns those values into undefined, so any post whose metaImagePath/heroImagePath is already absolute loses its RSS image and the newsletter card falls back to text-only.
Suggested fix
const cover = page.data.metaImagePath ?? page.data.heroImagePath;
const rel = cover ? withBlogBasePathForImageSrc(cover) : "";
const image =
rel.startsWith("/")
? `${baseUrl.replace(/\/$/, "")}${rel}`
- : undefined;
+ : /^https?:\/\//i.test(rel)
+ ? rel
+ : undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/blog/src/lib/rss.ts` around lines 28 - 33, The image URL logic discards
already-absolute cover URLs because it only handles rel starting with "/" —
change the conditional in the image resolution code (variables cover, rel, image
and helper withBlogBasePathForImageSrc) so that: if rel starts with "/" then
prefix baseUrl (baseUrl + rel), else if rel already is an absolute URL (starts
with "http://", "https://" or protocol-relative "//") keep rel as-is, otherwise
fall back to undefined; update the expression that computes image to implement
this three-way check so absolute URLs are preserved.
Comment on lines
+53
to
+54
| http://localhost:3002 http://127.0.0.1:3002 | ||
| https://www.prisma.io https://prisma.io |
Contributor
There was a problem hiding this comment.
Keep loopback image origins out of the production CSP.
These headers are applied to /:path*, so every deployed page now permits localhost:3002 and 127.0.0.1:3002 in img-src. That broadens the site-wide CSP for no production benefit and weakens loopback isolation. Please gate these origins behind a development-only branch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/site/next.config.mjs` around lines 53 - 54, The CSP currently includes
loopback image origins ("http://localhost:3002" and "http://127.0.0.1:3002")
unconditionally; update the headers generation (the exported headers function /
contentSecurityPolicy variable in next.config.mjs) to append those two origins
to the img-src only in development (e.g. when process.env.NODE_ENV ===
'development' or a DEV flag), and ensure the production branch/build path
constructs the img-src without those loopback origins so deployed pages do not
permit localhost or 127.0.0.1.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Public API
Chores