-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
feat(vue-query): add support for getters in query keys #7608
feat(vue-query): add support for getters in query keys #7608
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit b2ed0fc:
|
☁️ Nx Cloud ReportCI is running/has finished running commands for commit dc3b6cc. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 2 targetsSent with 💌 from NxCloud. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7608 +/- ##
===========================================
+ Coverage 44.46% 71.42% +26.96%
===========================================
Files 185 19 -166
Lines 7049 462 -6587
Branches 1549 119 -1430
===========================================
- Hits 3134 330 -2804
+ Misses 3552 102 -3450
+ Partials 363 30 -333
|
This could improve ergonomics, but solution is insufficient. QueryKey can be infinitely nested structure, so checking only the top level of QueryKey would not resolve deeply nested getters. This in turn will break query key serialization. We would need to create a variant of |
Ok thank you for your feedback on this. I'll look into how I can conditionally deep unwrap getters specifically for the query key. |
…ttipirneni/query into feat/vue-query-key-getters
@DamianOsipiuk I think I've managed to get nested getter functions to work. The new and old tests are passing so I (assume?) nothing broke in the process. Basically I added a I'm pretty confident useQuery will work as expected, I just wanted to verify with you if the changes to useQueries are ok. |
This is a based on the discussion I found in #5990. According to the author of the PR which made
enabled
a getter, they stated they didn't find a use case for query keys so it was not implemented.Personally, I have found many use cases for getters inside of query keys. Commonly, I'll define composables for my API and then have them accept reactive variables like so:
When I use these composable I often have to make intermediate
computed
variables to keep the data from props reactive:With this PR the we don't need intermediate
computed
properties to do this, you can write the code using getters instead:This also makes composables like the one I defined above fall more into line with the recommended practices for vue composables https://vuejs.org/guide/reusability/composables#input-arguments.
CC @DamianOsipiuk