-
Notifications
You must be signed in to change notification settings - Fork 367
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
refactor: [M3-7437] - Prepare Profile Queries for React Query v5 #9991
refactor: [M3-7437] - Prepare Profile Queries for React Query v5 #9991
Conversation
Coverage Report: ✅ |
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.
Profile loads ✅
Grants load for restricted users ✅
SSH keys load ✅
App tokens load ✅
PATs load ✅
@@ -1609,6 +1609,11 @@ | |||
resolved "https://registry.yarnpkg.com/@bcoe/v8-coverage/-/v8-coverage-0.2.3.tgz#75a2e8b51cb758a7553d6804a5932d7aace75c39" | |||
integrity sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw== | |||
|
|||
"@bnussman/query-key@0.0.7": |
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.
Should we create this as a package under the Linode/Akamai org?
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.
That's the plan! 📦 I will go over this package in the next cafe and we can proceed based on how the cafe discussion goes.
Verified:
|
queryFn: () => getPersonalAccessTokens(params, filter), | ||
queryKey: [params, filter], | ||
}), | ||
profile: { |
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.
I think this might cause conflicts with my current changes since getProfile now takes an optional header argument.
I think a simple fix would be:
queryFn: getProfile,
--> queryFn: () => getProfile(),
But I can even add this in my PR if this is merged before that
LGTM 🚀 Its def cleaner and more consistent, great job |
Description 📝
Changes of plans: No feature branch for the React Query update project. We will do all of the required changes first (merging to develop) then bump the React Query version last. This will eliminate the need for a feature branch.
Changes 🔄
Array
(required for React Query v4)How to test 🧪
Important
These code changes should result in no changes to the functionality/behavior of Cloud Manager. It is simply a migration to newer patterns.
Verification steps
As an Author I have considered 🤔