updates for new forkers #9
Conversation
WalkthroughRemoved Notion-related env entries from .env.example. Added posts-cache.json to .gitignore. Updated README setup wording and pnpm note. Added dotenv dependency. In src/lib/notion.ts, conditionally load env with dotenv for local dev, set explicit Notion API version, and added a ts-ignore in fetchPublishedPosts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App Startup
participant Env as dotenv (local)
participant Proc as process.env
participant Notion as Notion Client
participant API as Notion API
App->>Proc: Check VERCEL / NOTION_TOKEN
alt Local dev and NOTION_TOKEN missing
App->>Env: config({ path: .env.local/.env })
Env-->>Proc: Populate env vars
else Hosted or token present
App-->>Proc: Skip dotenv
end
App->>Notion: new Client({ auth: NOTION_TOKEN, notionVersion: "2022-06-28" })
App->>Notion: fetchPublishedPosts()
Note right of App: ts-ignore applied for query typing
Notion->>API: query database for published posts
API-->>Notion: Results
Notion-->>App: Posts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/lib/notion.ts (2)
1-6: Consider simplifying the dotenv loading pattern.The conditional environment loading works but could be simplified. The explicit path resolution with
path.resolve(process.cwd(), '.env.local')is redundant since dotenv naturally looks for.env.localin the current directory when using the defaultpathoption.Consider this simpler approach:
-import dotenv from 'dotenv'; -import path from 'path'; - -if (!process.env.VERCEL && !process.env.NOTION_TOKEN) { - dotenv.config({ path: path.resolve(process.cwd(), '.env.local') }); -} +import dotenv from 'dotenv'; + +if (!process.env.VERCEL && !process.env.NOTION_TOKEN) { + dotenv.config({ path: '.env.local' }); +}Alternatively, omit the path entirely since dotenv automatically searches for
.env.local:-import dotenv from 'dotenv'; -import path from 'path'; - -if (!process.env.VERCEL && !process.env.NOTION_TOKEN) { - dotenv.config({ path: path.resolve(process.cwd(), '.env.local') }); -} +import dotenv from 'dotenv'; + +if (!process.env.VERCEL && !process.env.NOTION_TOKEN) { + dotenv.config(); +}Based on learnings.
13-16: Update or confirm Notion API version
BumpnotionVersioninsrc/lib/notion.tsto"2025-09-03"or explicitly confirm retaining"2022-06-28"; if upgrading, adjust for reorganized/v1/data_sourcesendpoints per Notion’s upgrade guide.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.env.example(0 hunks).gitignore(1 hunks)README.md(2 hunks)package.json(2 hunks)src/lib/notion.ts(2 hunks)
💤 Files with no reviewable changes (1)
- .env.example
🔇 Additional comments (5)
.gitignore (1)
16-18: LGTM!The addition of
posts-cache.jsonto.gitignoreis appropriate. Cache files should not be committed to version control, and the comment clearly explains that it's regenerated on each build.package.json (2)
33-33: LGTM!The addition of
dotenvas a runtime dependency correctly supports the local development environment loading implemented insrc/lib/notion.ts. Based on learnings, version 17.2.3 is the latest stable release.
57-57: Minor observation:tw-animate-cssreordering.This dependency appears to have been removed and re-added (same version), which is a cosmetic no-op. This doesn't affect functionality.
README.md (2)
22-22: LGTM!The note about downloading pnpm is helpful for new users who may not have it installed.
39-39: LGTM!The documentation updates improve clarity and align with current Notion API terminology:
- Updated integration naming guidance for easier searching
- "Internal Integration Secret" is more precise than "token"
- "Enter your integration name" is clearer than the previous instruction
Also applies to: 43-43, 49-49
|
|
||
| export async function fetchPublishedPosts() { | ||
| // This function is now intended to be used only by the caching script. | ||
| // @ts-ignore - bypass TypeScript error with v4 SDK types |
There was a problem hiding this comment.
Remove the @ts-ignore directive and fix the underlying type issue.
Using @ts-ignore suppresses TypeScript's type checking, which defeats the purpose of using TypeScript and can hide legitimate type errors. The comment mentions "v4 SDK types" but doesn't explain why bypassing types is safe or necessary.
Instead of suppressing the error, investigate and resolve the underlying type mismatch. Possible solutions:
- Cast the response appropriately if you're confident about the runtime types
- Update the SDK if there's a version mismatch
- Define proper types that match the actual API response
If the SDK types are genuinely incorrect, consider:
- Opening an issue with the SDK maintainers
- Adding a detailed comment explaining the type mismatch and why the workaround is safe
- Using a type assertion instead of
@ts-ignoreto maintain some type safety
Example with type assertion:
const posts = await notion.databases.query({
database_id: process.env.NOTION_DATABASE_ID!,
filter: {
and: [
{
property: "Status",
status: {
equals: "Published",
},
},
],
},
sorts: [
{
property: "Published Date",
direction: "descending",
},
],
}) as any; // More explicit about intentional type bypass🤖 Prompt for AI Agents
In src/lib/notion.ts around line 63, remove the `// @ts-ignore` and fix the
underlying type mismatch by either importing and using the correct Notion SDK
types (or updating the SDK if versions are mismatched), or replace the ignore
with a narrow type assertion that matches the actual response shape (define an
interface for the expected query result and cast the response to that
interface), ensuring you keep type safety and add a short comment if the
assertion is intentional; if the SDK types are truly wrong, note that in a TODO
and consider filing an upstream issue.
1 participant
Summary by CodeRabbit
Documentation
Chores