-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(core): added undefined type successNotification
and errorNotification
#6327
Conversation
🦋 Changeset detectedLatest commit: 2e25480 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Anonymous961 thank you for the PR! Just checked it and looks like the change is correct but made in the wrong place 😅
undefined
should be added to the return values of the function type of the union 😅
Sharing the lines below:
refine/packages/core/src/contexts/notification/types.ts
Lines 15 to 19 in 5a87542
| (( | |
data?: TData, | |
values?: TVariables, | |
resource?: string, | |
) => OpenNotificationParams | false); |
refine/packages/core/src/contexts/notification/types.ts
Lines 28 to 32 in 5a87542
| (( | |
error?: TError, | |
values?: TVariables, | |
resource?: string, | |
) => OpenNotificationParams | false); |
Line 19 and Line 32 should have undefined
.
successNotification
and errorNotification
already has ?
in their keys which makes them accept undefined
already.
Let me know if you can apply the changes 🙏
Added undefined type to errorNotification and successNotification.
5a87542
to
6dc730d
Compare
Ohh sorry my bad 😅 I just made the changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the update @Anonymous961 🙏 This PR will be included in our next release 🚀
Added undefined type to errorNotification and successNotification.
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
What is the new behavior?
fixes #6270
Notes for reviewers
I followed the instructions provided by @aliemir in the issue discussion. Please let me know if any further changes are needed or if there are additional aspects I should address.