Merged
Conversation
The groupListItems function was missing an else clause to handle non-list blocks (paragraphs, headings, code, etc.), causing it to never increment the loop index when encountering these blocks. This manifested as a build hang when generating the RSS feed, as it was the first place to call getBlockChildren during the build.
Removed test for child_page block transformation as support for child_page blocks was previously removed from the schema.
Added test that directly validates the regex pattern matches all
rehype-pretty-code inline language indicator formats ({:js}, {:.fp})
and correctly excludes normal code endings.
Renamed function to better describe its purpose and added comprehensive
JSDoc explaining:
- What rehype-pretty-code language indicators are
- Why they need to be stripped for RSS feeds
- Examples of both language ({:js}) and class ({:.fp}) formats
Added comprehensive integration test that generates the actual RSS feed and validates: - XML declaration and RSS 2.0 structure - Well-formedness via matching opening/closing tag counts - Channel metadata (title, description, link, language) - Item structure (title, link, guid, pubDate) - Featured image rendering in content - HTML content rendering (headings, formatting) - CDATA wrapping for special characters Also configured test file to use happy-dom environment for DOMParser support (though final implementation uses regex validation instead due to happy-dom's limited namespace support).
Make pagination navigation more accessible by:
- Adding decorative prop to Emoji component for non-semantic emoji
- Marking directional emoji as decorative (aria-hidden)
- Hiding redundant direction text ("previous"/"next") from screen readers
The post titles themselves now serve as the accessible names for the links,
which is more concise and clearer for screen reader users.
Update test to verify the new accessible name structure where post titles
serve as the accessible names (with non-breaking spaces) rather than the
directional text ("previous"/"next").
Remove unused imports across cache, Notion, and app modules.
Export schema to allow test validation of cached Cloudinary metadata structure.
Apply consistent code formatting to HTML renderer functions.
Apply consistent code formatting to HTML renderer test file.
Fix CDATA regex pattern to properly match multiline content using [\s\S] instead of . with /s flag for better cross-platform compatibility.
Add feedId field extraction in transformNotionPageToPost and update all related tests to include the Feed ID property in mock data. Also remove unused PostSchema import.
Update getPosts and integration tests to include Feed ID property in all mock Notion pages for consistency with the updated schema.
Apply consistent markdown formatting to the RSS feed implementation spec.
Adds explicit check for undefined data field after Zod validation to ensure cache file structure validation is robust. This handles the edge case where z.unknown() might allow undefined in some environments.
There was a problem hiding this comment.
Pull request overview
This PR adds RSS feed functionality at /rss.xml with full post content rendered from Notion blocks, improves feed reader accessibility, and enhances screen reader experience for pagination navigation.
- Implements RSS 2.0 feed with all published blog posts including rendered HTML content
- Adds auto-discovery link tag for feed readers
- Makes pagination emojis and direction labels decorative to improve accessible names
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
app/rss.xml/route.ts |
New RSS feed route handler that generates RSS 2.0 XML with post content |
app/rss.xml/route.test.ts |
Comprehensive test coverage for RSS feed generation |
io/notion/renderBlocksToHtml.ts |
New HTML renderer for Notion blocks to support RSS feed content |
io/notion/renderBlocksToHtml.test.ts |
Test coverage for HTML rendering logic |
io/notion/schemas/post.ts |
Adds optional feedId field for historical feed stability |
io/notion/getPosts.ts |
Updates to include feedId in post transformations |
io/notion/getPost.ts |
Updates to include feedId in post transformations |
io/notion/getBlockChildren.ts |
Bug fix for non-list block handling |
ui/emoji.tsx |
Adds decorative prop for accessibility |
ui/nav/pagination.tsx |
Uses decorative emoji and hides direction text from screen readers |
app/layout.tsx |
Adds RSS auto-discovery link tag |
package.json |
Adds feed library dependency |
| Test files | Updates test data to include new feedId field |
| Cleanup files | Removes unused imports |
Comments suppressed due to low confidence (1)
app/rss.xml/route.ts:1
- The RSS feed route lacks test coverage for error handling when getPosts fails. Consider adding a test case that verifies the behavior when getPosts returns an error result.
import { Feed } from 'feed'
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+65
to
+68
| } else { | ||
| // For non-list blocks, add them directly | ||
| grouped.push(block) | ||
| i++ |
There was a problem hiding this comment.
The loop increment i++ is placed inside the else block, but it should be incremented for both list and non-list blocks. Without incrementing i in the list block cases (bulleted_list and numbered_list), the loop will iterate over the same block infinitely when a list is encountered.
io/notion/renderBlocksToHtml.ts
Outdated
| * @returns HTML string with all language indicators removed | ||
| */ | ||
| function stripRehypePrettyCodeLanguageIndicators(html: string): string { | ||
| const rehypePrettyCodeInlineLangIndicator = /{:\\.?\\w+}<\/code>/g |
There was a problem hiding this comment.
The regex pattern contains unnecessary escaped backslashes. In a regular expression literal, \\. should be \. and \\w should be \w. The pattern should be /{:\.?\w+}<\/code>/g.
Suggested change
| const rehypePrettyCodeInlineLangIndicator = /{:\\.?\\w+}<\/code>/g | |
| const rehypePrettyCodeInlineLangIndicator = /{:\.?\w+}<\/code>/g |
app/layout.tsx
Outdated
| description: 'Software engineer helping scientists discover new medicines at Recursion.', | ||
| alternates: { | ||
| types: { | ||
| 'application/rss+xml': '/rss.xml/', |
There was a problem hiding this comment.
The RSS feed URL includes a trailing slash (/rss.xml/), but RSS feed URLs conventionally do not include a trailing slash. Consider using /rss.xml instead for better compatibility with feed readers.
Suggested change
| 'application/rss+xml': '/rss.xml/', | |
| 'application/rss+xml': '/rss.xml', |
- Fix regex pattern: remove unnecessary escaped backslashes - Remove trailing slash from RSS feed URL for better feed reader compatibility
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
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.
✅ What
/rss.xmlwith all published blog posts<link>tag in site<head>so feed readers can find the feed🤔 Why
👩🔬 How to validate
http://localhost:3000/rss.xml<enclosure>tagshttp://localhost:3000and search forapplication/rss+xml<link rel="alternate"pointing to/rss.xml