-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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): protect useLogin
default success handler from custom onSuccess
overrides
#5889
base: master
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 8ed7ac9 The changes in this PR will be included in the next version bump. 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 |
165b069
to
167e100
Compare
useLogin
useLogin
default success handler from custom onSuccess
overrides
54755f0
to
8ed7ac9
Compare
Hey @MohammadxAli, thank you for the PR! As you can see from the line here
We're omitting On all other Refine hooks, we've omitted I'll also share this comment in the issue, let us know if you're willing to work on the further changes 🙏 |
Hey 👋 Thanks for looking into this.
That makes sense, however seems like even though the Code_1naxEN4HAW.mp4There's no errors and even going to definition takes me to react-query typings. It's super weird though :) I have no idea why's that the case. At the same time, I believe allowing a custom
I don't mind working on it :) But I'm not familiar with the overall codebase so I might be asking some questions around to find my way. I agree with your suggestion on adding it to all of refine hooks implementations. As a react-query user, I reach for |
We'd love to help if you face any issues while working on this. If you want to continue working on this branch, let's convert it to draft. I can try to review your work while it's in progress and we can discuss here about any decisions to make or help if you face any issues 🚀 I'll also try to look what went wrong with the omit in |
Awesome! 👍 Some questions though:
|
Actually, you can check for hooks which omits these callbacks in types. If you have any issues locating them or cannot decide, I can try coming up with a list but cannot promise today or this week 😅
Single PR will be enough I think. If you think you've made some additional changes that needs to be addressed in a separate PR we can always discuss it here.
Yeah, it will be great actually 😅 |
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
What is the new behavior?
fixes #5888
Notes for reviewers