Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds the Sonner toast library, a themed Toaster wrapper with custom icons and styles, Storybook stories demonstrating many toast patterns/positions, and exposes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
…onal actions and descriptions
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/ui/sonner.tsx (1)
16-35: Mergeicons/styledefaults instead of letting props replace them wholesale.Because
...propsis applied on Line 34, passingiconsorstyleoverrides the wrapper defaults entirely. If this wrapper is intended to preserve design-system defaults, merge those props.Proposed refactor
-const Toaster = ({ ...props }: ToasterProps) => { +const Toaster = ({ icons, style, theme, ...props }: ToasterProps) => { const theme = useThemeOptional(); const mode = theme?.mode ?? "system"; return ( <Sonner - theme={mode as ToasterProps["theme"]} + theme={theme ?? (mode as ToasterProps["theme"])} className="toaster group" icons={{ success: <CircleCheckIcon className="size-4" />, info: <InfoIcon className="size-4" />, warning: <TriangleAlertIcon className="size-4" />, error: <OctagonXIcon className="size-4" />, loading: <Loader2Icon className="size-4 animate-spin" />, + ...icons, }} style={ { "--normal-bg": "var(--popover)", "--normal-text": "var(--popover-foreground)", "--normal-border": "var(--border)", "--border-radius": "var(--radius)", + ...(style as React.CSSProperties), } as React.CSSProperties } {...props} /> ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/sonner.tsx` around lines 16 - 35, The Sonner wrapper currently lets incoming props.icons and props.style fully replace the defaults because {...props} is spread last; update the component in src/components/ui/sonner.tsx to merge defaults with incoming props instead: define the defaultIcons and defaultStyle (matching the existing icons and style object) and pass icons={...defaultIcons, ...(props?.icons)} and style={{...defaultStyle, ...(props?.style)}} (or use Object.assign) when rendering <Sonner ... /> so callers can override specific keys without discarding design-system defaults.src/components/ui/Sonner.stories.tsx (1)
157-160: UsePromisedirectly instead ofwindow.Promise.Line 158 can use the global
Promiseconstructor directly; this keeps the story less browser-coupled and more idiomatic.Proposed tweak
- new window.Promise<{ name: string }>((resolve) => + new Promise<{ name: string }>((resolve) => setTimeout(() => resolve({ name: "Sonner" }), 2000) ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/Sonner.stories.tsx` around lines 157 - 160, Replace the browser-coupled constructor usage "new window.Promise<{ name: string }>(...)" with the global Promise constructor by removing "window." so the story uses "new Promise<{ name: string }>(...)" instead; locate the arrow function that returns "new window.Promise" in Sonner.stories (the async story helper) and update it to use Promise to keep the code idiomatic and environment-agnostic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 78: Update the package.json peerDependencies so they match Sonner 2.x's
React requirement: in package.json's "peerDependencies" replace the current
react and react-dom ranges that allow ">=17.0.0" with ranges that require React
18+ (e.g., ">=18.0.0" or the equivalent semver like "^18.0.0 || ^19.0.0") so the
declared peers align with the installed "sonner": "^2.0.7" constraint.
---
Nitpick comments:
In `@src/components/ui/Sonner.stories.tsx`:
- Around line 157-160: Replace the browser-coupled constructor usage "new
window.Promise<{ name: string }>(...)" with the global Promise constructor by
removing "window." so the story uses "new Promise<{ name: string }>(...)"
instead; locate the arrow function that returns "new window.Promise" in
Sonner.stories (the async story helper) and update it to use Promise to keep the
code idiomatic and environment-agnostic.
In `@src/components/ui/sonner.tsx`:
- Around line 16-35: The Sonner wrapper currently lets incoming props.icons and
props.style fully replace the defaults because {...props} is spread last; update
the component in src/components/ui/sonner.tsx to merge defaults with incoming
props instead: define the defaultIcons and defaultStyle (matching the existing
icons and style object) and pass icons={...defaultIcons, ...(props?.icons)} and
style={{...defaultStyle, ...(props?.style)}} (or use Object.assign) when
rendering <Sonner ... /> so callers can override specific keys without
discarding design-system defaults.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5273f740-d87b-4345-b266-5432eb123c5f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.jsonsrc/components/ui/Sonner.stories.tsxsrc/components/ui/index.tssrc/components/ui/sonner.tsxsrc/index.ts
| "input-otp": "^1.4.2", | ||
| "lucide-react": "^0.563.0", | ||
| "recharts": "^2.15.4", | ||
| "sonner": "^2.0.7", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Sonner 2.0.7 peerDependencies ==="
npm view sonner@2.0.7 peerDependencies --json
echo ""
echo "=== Current library peerDependencies ==="
python - <<'PY'
import json
with open("package.json") as f:
pkg = json.load(f)
print(json.dumps(pkg.get("peerDependencies", {}), indent=2))
PYRepository: getdokan/plugin-ui
Length of output: 295
Update peerDependencies to reflect Sonner's React 18+ requirement.
Sonner 2.0.7 requires react@^18.0.0 || ^19.0.0 and react-dom@^18.0.0 || ^19.0.0, but the current peer ranges declare >=17.0.0. This mismatch creates install conflicts for consumers using React 17. Update peer ranges to >=18.0.0:
Proposed fix
"peerDependencies": {
- "react": ">=17.0.0",
- "react-dom": ">=17.0.0"
+ "react": ">=18.0.0",
+ "react-dom": ">=18.0.0"
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 78, Update the package.json peerDependencies so they
match Sonner 2.x's React requirement: in package.json's "peerDependencies"
replace the current react and react-dom ranges that allow ">=17.0.0" with ranges
that require React 18+ (e.g., ">=18.0.0" or the equivalent semver like "^18.0.0
|| ^19.0.0") so the declared peers align with the installed "sonner": "^2.0.7"
constraint.
Summary by CodeRabbit
New Features
Documentation
Public API