Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
WalkthroughAdds a new framework-agnostic package Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 (1)
packages/react-sdk/src/types.ts (1)
25-25: Consider using a regular import for consistency.The inline import syntax works but is less conventional. For better readability and consistency with the rest of the file, consider importing
Postat the top.🔎 Proposed refactor
import type { ReactNode } from "react"; -import type { PostTag } from "@changespage/core"; +import type { Post, PostTag } from "@changespage/core";Then update line 25:
export interface ChangelogPostProps { - post: import("@changespage/core").Post; + post: Post; locale?: string; children: (props: ChangelogPostRenderProps) => ReactNode; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
packages/core-sdk/package.json(1 hunks)packages/core-sdk/src/client.ts(1 hunks)packages/core-sdk/src/index.ts(1 hunks)packages/core-sdk/src/types.ts(1 hunks)packages/core-sdk/src/utils.ts(2 hunks)packages/core-sdk/tsconfig.json(1 hunks)packages/react-sdk/package.json(1 hunks)packages/react-sdk/src/components/ChangelogPost.tsx(1 hunks)packages/react-sdk/src/hooks/usePosts.ts(1 hunks)packages/react-sdk/src/index.ts(1 hunks)packages/react-sdk/src/types.ts(2 hunks)
🔇 Additional comments (12)
packages/core-sdk/tsconfig.json (1)
1-21: LGTM! Well-configured TypeScript setup for a library package.The configuration is appropriate for a modern TypeScript library with proper declaration file generation, strict mode, and ESNext module support. The
moduleResolution: "bundler"option is compatible with TypeScript 5.3.3 specified in the package.json.packages/core-sdk/src/utils.ts (2)
19-22: LGTM! Robust validation for date formatting.The added validation checks properly handle edge cases:
- Null, empty, or whitespace-only strings
- Invalid date strings that cannot be parsed
Returning an empty string for invalid dates is a sensible default for UI display.
31-36: LGTM! Consistent validation pattern.The validation logic matches the approach in
formatDate, ensuring that only valid Date objects are returned. Returningnullfor invalid dates is appropriate for the API contract.packages/react-sdk/package.json (1)
20-22: LGTM! Correct monorepo dependency setup.The workspace dependency on
@changespage/coreis properly configured using theworkspace:*protocol, which is the standard approach for monorepo package dependencies.packages/core-sdk/package.json (1)
1-34: LGTM! Standard package configuration for a TypeScript library.The package manifest is properly configured with:
- ESM module type and exports
- TypeScript build pipeline using tsc
- Public access for npm publishing
- Appropriate metadata and repository info
packages/core-sdk/src/types.ts (1)
1-34: LGTM! Comprehensive and well-structured type definitions.The type definitions provide a solid foundation for the core SDK:
- Clear PostTag union type
- Complete Post interface with all necessary fields
- Simple, focused interfaces for configuration and options
- Well-defined client interface including the new
getPinnedPostmethodpackages/react-sdk/src/hooks/usePosts.ts (1)
4-4: LGTM! Clean migration to core types.The import has been correctly updated to use types from the new
@changespage/corepackage, aligning with the PR's objective to centralize shared types.packages/react-sdk/src/components/ChangelogPost.tsx (1)
1-1: LGTM! Clean migration to core utilities.The date utility imports have been correctly updated to use the centralized implementations from
@changespage/core.packages/core-sdk/src/client.ts (1)
76-80: LGTM! Method properly exposed in client interface.The
getPinnedPostmethod is correctly added to the returned client object, matching theChangesPageClientinterface definition in types.ts.packages/core-sdk/src/index.ts (1)
1-10: LGTM! Clean barrel export structure.The core SDK public API is well-organized with clear separation between client creation, utilities, and type exports. This establishes a solid foundation for the new
@changespage/corepackage.packages/react-sdk/src/types.ts (1)
4-11: LGTM! Proper type re-exports from core package.The type exports correctly delegate to
@changespage/core, maintaining backward compatibility while centralizing type definitions.packages/react-sdk/src/index.ts (1)
1-23: LGTM! Excellent refactoring to delegate to core package.The React SDK now properly re-exports core functionality from
@changespage/corewhile maintaining its own React-specific exports (components and hooks). This achieves the PR objective of separating core and React SDK functionality while maintaining backward compatibility.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/react-sdk/README.md (2)
60-62: Document the newgetPinnedPost()method.The PR introduces a
getPinnedPost()client method per the AI summary, but it's not documented in the README. Add documentation for this new public API method alongside the existinggetLatestPost()method to maintain API completeness.🔎 Suggested addition to document getPinnedPost()
### `client.getLatestPost()` Returns the most recent post or `null`. + +### `client.getPinnedPost()` + +Returns the pinned post or `null`.
64-68: Document migration path for breaking changes in<ChangelogPost>render props.The render prop interface has changed—
publicationDatereplaces the previous date properties, and per the enriched context,localehas been removed from props. These are breaking changes that should be clearly documented to help users migrate from older versions.Consider adding a brief migration section or inline note highlighting what changed to enable users to update their code.
🧹 Nitpick comments (2)
apps/web/pages/changelog.tsx (1)
52-60: Consider defensive error handling for invalid dates.The conditional rendering checks for the existence of
publicationDate, but if the value is an invalid date string,new Date(publicationDate).toLocaleDateString()may throw an error or return "Invalid Date".🔎 Suggested defensive date handling
- {publicationDate && ( + {publicationDate && !isNaN(new Date(publicationDate).getTime()) && ( <time className="text-sm text-gray-500 dark:text-gray-400"> {new Date(publicationDate).toLocaleDateString("en-US", { year: "numeric", month: "long", day: "numeric", })} </time> )}Alternatively, you could wrap the date parsing in a try-catch or create a helper function for safer date formatting.
packages/core-sdk/README.md (1)
42-50: Consider varying sentence structure for better readability.Three consecutive API descriptions begin with "Returns," which creates repetitive phrasing. Consider varying the sentence structure for improved documentation flow.
📝 Suggested rewording
-Returns `{ posts, totalCount, hasMore }` +Retrieves a paginated list of posts, returning `{ posts, totalCount, hasMore }`. ### `client.getLatestPost()` -Returns the most recent post or `null`. +Fetches the most recent post, or `null` if none exists. ### `client.getPinnedPost()` -Returns the pinned post or `null` if none is pinned. +Retrieves the currently pinned post, or `null` if no post is pinned.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/pages/changelog.tsx(1 hunks)packages/core-sdk/README.md(1 hunks)packages/core-sdk/src/index.ts(1 hunks)packages/core-sdk/src/utils.ts(1 hunks)packages/react-sdk/README.md(2 hunks)packages/react-sdk/src/components/ChangelogPost.tsx(1 hunks)packages/react-sdk/src/index.ts(1 hunks)packages/react-sdk/src/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/core-sdk/src/index.ts
- packages/react-sdk/src/types.ts
- packages/react-sdk/src/components/ChangelogPost.tsx
🧰 Additional context used
🪛 LanguageTool
packages/core-sdk/README.md
[style] ~49-~49: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... null. ### client.getPinnedPost() Returns the pinned post or null if none is pi...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (3)
packages/react-sdk/README.md (1)
28-28: API documentation correctly reflectspublicationDateproperty.The changes to the usage example and render prop documentation properly document the shift to
publicationDate, aligning with the core SDK migration. The implementation example demonstrates correct date handling withnew Date(publicationDate).toLocaleDateString().Also applies to: 31-31, 68-68
packages/react-sdk/src/index.ts (1)
1-18: The refactoring to re-export from@changespage/coreis complete and correct. All items from core that are re-exported match exactly with what the core package exports (createChangesPageClient,getTagLabel, and their associated types). No breaking changes were introduced.The original claim about
formatDateandparseDateutilities being removed is incorrect—these utilities do not exist in the codebase and were never exported, so there is no breaking change to document.Likely an incorrect or invalid review comment.
packages/core-sdk/src/utils.ts (1)
1-13: LGTM! Clean utility implementation.The
getTagLabelfunction is well-implemented with proper type safety and a sensible fallback using nullish coalescing. AllPostTagvalues (fix,new,improvement,announcement,alert) are covered in thetagLabelsmapping, enforced by theRecord<PostTag, string>type annotation.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.