Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Fixes toast styling selection by ensuring unsupported toast variant values (e.g. "success") fall back to the default toast styles instead of producing an unstyled/incorrect toast.
Changes:
- Extracted toast variant class definitions into a shared
VARIANT_CLASSESconstant and derivedToastVariantfrom it. - Added a runtime
isToastVariantguard and used it to resolve invalidvariantvalues to"default"before passing intocva.
| ); | ||
|
|
||
| function isToastVariant(value: unknown): value is ToastVariant { | ||
| return typeof value === "string" && value in VARIANT_CLASSES; |
There was a problem hiding this comment.
isToastVariant uses value in VARIANT_CLASSES, which is true for inherited Object prototype keys (e.g. "toString"). That makes the type guard unsound and can result in non-string classNames being passed into cva (e.g. Object.prototype.toString), producing broken class output. Use an own-property check instead (e.g. Object.prototype.hasOwnProperty.call(VARIANT_CLASSES, value) or an equivalent safe helper).
| return typeof value === "string" && value in VARIANT_CLASSES; | |
| return ( | |
| typeof value === "string" && | |
| Object.prototype.hasOwnProperty.call(VARIANT_CLASSES, value) | |
| ); |
There was a problem hiding this comment.
We should do something like !!VARIANT_CLASSES[value]
kirangadhave
left a comment
There was a problem hiding this comment.
🚀 approving, but we should address the copilot suggestion
| ); | ||
|
|
||
| function isToastVariant(value: unknown): value is ToastVariant { | ||
| return typeof value === "string" && value in VARIANT_CLASSES; |
There was a problem hiding this comment.
We should do something like !!VARIANT_CLASSES[value]
📝 Summary
Code_mode would call toast with
successkind, which is not supported at the moment. So, it would choose the wrong styles.This resolves to choose the
defaultstyle and adds some better typing.📋 Pre-Review Checklist
✅ Merge Checklist