-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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(query-core): add support for symbol in cache #7857
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit a7ad13c. 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 #7857 +/- ##
=========================================
+ Coverage 0 63.31% +63.31%
=========================================
Files 0 127 +127
Lines 0 4539 +4539
Branches 0 1266 +1266
=========================================
+ Hits 0 2874 +2874
- Misses 0 1437 +1437
- Partials 0 228 +228 |
sorry, symbols aren't supported in query keys because they are't json serializable, which is what we use for hashing. You can address that on your end by providing a |
But those aren't in query types. But in the cached values I didn't change the logic of the keys themselves (I saw a |
You can see here in the code: const key = queryKey()
const s = Symbol('s');
queryClient.setQueryData(key, { [s]: 1, a: 1 }) The symbol is not in the query key but in the data itself |
ah, sorry about that. So you basically changed the implementation of The reason why I'm hesitant to add this to the library is because once we go away from "must be json serializable", we have to (and already did get) many requests about Maps, Sets, Dates etc. superjson is generally really good at that. |
Makes sense, I'll check that then |
I noticed when I was using symbols as keys in the value of objects with
queryCache.setQueryData()
that keys containing symbol were sometimes stripped outThis PR addresses this by using
Object.getOwnPropertySymbols()
to iterate over symbol keys in the cacheI added new tests & ran test before and after my changes and they indeed fail on before, and pass after