-
Notifications
You must be signed in to change notification settings - Fork 77
feat: adds react-query automatic proxy package #2440
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
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a new package Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Consumer as Consumer Component
participant Proxy as Hooks Proxy
participant RQ as React Query
participant SDK as SDK Method
note over Consumer,Proxy `#D3E4CD`: 1) Consumer accesses proxy (e.g., api.user.get)
Consumer->>Proxy: access api.user.get
note right of Proxy: Proxy returns HooksProxy with getKey/useQuery/useMutation
opt useQuery flow
Consumer->>Proxy: call useQuery(input, opts)
Proxy->>Proxy: compute queryKey (path + inputToKey(input))
Proxy->>RQ: call useQuery({ queryKey, queryFn })
RQ->>SDK: invoke queryFn -> SDK.user.get(input)
SDK-->>RQ: return result
RQ-->>Consumer: provide data / status
end
opt useMutation flow
Consumer->>Proxy: call useMutation(opts)
Proxy->>Proxy: compute mutationKey (path)
Proxy->>RQ: call useMutation({ mutationKey, mutationFn })
Consumer->>RQ: trigger mutate(input)
RQ->>SDK: call mutationFn -> SDK.user.update(input)
SDK-->>RQ: return result
RQ-->>Consumer: mutation result / status
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
| "link": true | ||
| }, | ||
| "node_modules/@akashnetwork/provider-proxy": { | ||
| "resolved": "apps/provider-proxy", |
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.
🔄 Carefully review the package-lock.json diff
Resolve the comment if everything is ok
+ node_modules/@akashnetwork/react-query-proxy link:packages/react-query-proxy
+ packages/react-query-proxy 1.0.0
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
02225eb to
dcc5410
Compare
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.
Actionable comments posted: 7
🧹 Nitpick comments (1)
packages/react-query-proxy/package.json (1)
4-4: Add a description for the package.The
descriptionfield is empty. Adding a meaningful description will help users understand the package's purpose.🔎 Proposed fix
- "description": "", + "description": "Wrap any async service into fully-typed TanStack React Query hooks at runtime",
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
packages/react-query-proxy/README.mdpackages/react-query-proxy/package.jsonpackages/react-query-proxy/src/createProxy/createProxy.spec.tspackages/react-query-proxy/src/createProxy/createProxy.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
packages/react-query-proxy/src/createProxy/createProxy.spec.tspackages/react-query-proxy/src/createProxy/createProxy.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
packages/react-query-proxy/src/createProxy/createProxy.spec.ts
🧠 Learnings (2)
📚 Learning: 2025-11-25T17:45:52.965Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-11-25T17:45:52.965Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files. The `setup` function must be at the bottom of the root `describe` block, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
Applied to files:
packages/react-query-proxy/src/createProxy/createProxy.spec.ts
📚 Learning: 2025-11-25T17:45:44.790Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-11-25T17:45:44.790Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test
Applied to files:
packages/react-query-proxy/src/createProxy/createProxy.spec.ts
🧬 Code graph analysis (1)
packages/react-query-proxy/src/createProxy/createProxy.spec.ts (1)
packages/react-query-proxy/src/createProxy/createProxy.ts (1)
createProxy(4-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: validate (apps/log-collector) / validate-unsafe
- GitHub Check: validate (apps/stats-web) / validate-unsafe
- GitHub Check: validate (apps/api) / validate-unsafe
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
- GitHub Check: Analyze (javascript-typescript)
| }, | ||
| "license": "Apache-2.0", | ||
| "author": "Akash Network", | ||
| "main": "src/index.ts", |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if src/index.ts exists and verify exports in the react-query-proxy package
if [ -f "packages/react-query-proxy/src/index.ts" ]; then
echo "=== src/index.ts exists ==="
cat -n "packages/react-query-proxy/src/index.ts"
else
echo "ERROR: src/index.ts does not exist"
exit 1
fiRepository: akash-network/console
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Check the structure of the react-query-proxy package
echo "=== Directory structure of packages/react-query-proxy ==="
ls -la packages/react-query-proxy/
echo ""
echo "=== Check if src directory exists ==="
if [ -d "packages/react-query-proxy/src" ]; then
echo "src directory exists. Contents:"
ls -la packages/react-query-proxy/src/
else
echo "src directory does NOT exist"
fi
echo ""
echo "=== Check package.json main field ==="
grep -A 2 -B 2 '"main"' packages/react-query-proxy/package.jsonRepository: akash-network/console
Length of output: 808
🏁 Script executed:
#!/bin/bash
# Check what's in the createProxy directory
echo "=== Contents of src/createProxy ==="
find packages/react-query-proxy/src -type f
echo ""
echo "=== Full directory tree ==="
tree packages/react-query-proxy/src/ 2>/dev/null || find packages/react-query-proxy/src -type f -o -type d | sortRepository: akash-network/console
Length of output: 373
Create src/index.ts to properly export the public API.
The main field in package.json points to src/index.ts, but this file does not exist. The actual implementation is in src/createProxy/createProxy.ts. Create src/index.ts to export createProxy and related types so the package can be imported correctly.
🤖 Prompt for AI Agents
In packages/react-query-proxy/package.json around line 11, package.json's "main"
points to src/index.ts but that file is missing; create src/index.ts that
re-exports the public API from src/createProxy/createProxy.ts (export
createProxy and any related types/interfaces) so imports resolve correctly;
ensure the file uses named exports matching the implementation and update any
type-only exports if needed.
| jest.mock("@tanstack/react-query", () => ({ | ||
| useQuery: jest.fn(), | ||
| useMutation: jest.fn() | ||
| })); |
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.
Replace jest.mock() with jest-mock-extended.
The coding guidelines specify: "Don't use jest.mock() in test files. Instead, use jest-mock-extended to create mocks and pass mocks as dependencies to the service under test."
Based on coding guidelines for test files.
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| useQueryMock.mockReturnValue({ data: undefined, isLoading: true }); | ||
| useMutationMock.mockReturnValue({ mutate: jest.fn(), isPending: false }); | ||
| }); |
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.
🛠️ Refactor suggestion | 🟠 Major
Move mock initialization logic into the setup function.
The coding guidelines specify using a setup function instead of beforeEach. The mock reset and return value setup should be moved into the setup function.
🔎 Proposed refactor
Remove the beforeEach block and update the setup function:
- beforeEach(() => {
- jest.clearAllMocks();
- useQueryMock.mockReturnValue({ data: undefined, isLoading: true });
- useMutationMock.mockReturnValue({ mutate: jest.fn(), isPending: false });
- });Then update the setup function:
function setup() {
+ jest.clearAllMocks();
+ useQueryMock.mockReturnValue({ data: undefined, isLoading: true });
+ useMutationMock.mockReturnValue({ mutate: jest.fn(), isPending: false });
+
const sdk = createSdk();
const proxy = createProxy(sdk);
return { sdk, proxy };
}Based on coding guidelines for test files.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/react-query-proxy/src/createProxy/createProxy.spec.ts around lines
32 to 36, the test file uses a beforeEach to clear mocks and set mockReturnValue
for useQueryMock and useMutationMock, but guidelines require these
initializations to be in the setup function; remove the beforeEach block and
move jest.clearAllMocks(), useQueryMock.mockReturnValue({ data: undefined,
isLoading: true }), and useMutationMock.mockReturnValue({ mutate: jest.fn(),
isPending: false }) into the existing setup function so the setup routine
handles mock reset and default return values before each test.
| import type { QueryKey, UseMutationOptions, UseMutationResult, UseQueryOptions, UseQueryResult } from "@tanstack/react-query"; | ||
| import { useMutation, useQuery } from "@tanstack/react-query"; | ||
|
|
||
| export function createProxy<T extends Record<string, any>>(object: T, proxyOptions?: CreateProxyOptions): RecursiveHooksProxy<T> { |
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.
Replace any types with proper TypeScript types.
The coding guidelines state: "Never use type any or cast to type any. Always define the proper TypeScript types."
This file uses any extensively:
- Lines 4, 15:
Record<string, any> - Lines 32, 50, 61:
(target as any)[prop] - Lines 77, 80:
(...args: any[]) => any
Consider using:
Record<string, unknown>instead ofRecord<string, any>- Proper type guards or generics instead of
as anycasts - More specific function signatures for type parameters
As per coding guidelines.
Also applies to: 15-15, 32-32, 50-50, 61-61, 77-77, 80-80
🤖 Prompt for AI Agents
In packages/react-query-proxy/src/createProxy/createProxy.ts around lines
4,15,32,50,61,77,80, replace all uses of the any type and any-casts with precise
TypeScript types: change Record<string, any> to Record<string, unknown> (or
better, use T extends Record<string, unknown>), replace (target as any)[prop]
with typed indexing using prop as keyof T & string or T[keyof T] after
narrowing, introduce type guards (isFunction, isObject) to narrow unknown values
instead of casting, and change function signatures from (...args: any[]) => any
to (...args: unknown[]) => unknown (or a generic Fn<Args extends unknown[], R =
unknown>) so calls and returns are typed; ensure you propagate specific generics
for method results and use Exclude/NonNullable where appropriate to avoid unsafe
casts.
| const mutationKey = fullPath; | ||
| if (options?.mutationKey) { | ||
| mutationKey.push(...(options.mutationKey as PropertyKey[])); |
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.
Critical bug: mutationKey mutates the shared fullPath array.
Line 54 assigns mutationKey = fullPath by reference, then line 56 pushes additional keys to it. This mutates the original fullPath array, corrupting it for all subsequent calls and cached proxies.
🔎 Proposed fix
- useMutation: options => {
- const mutationKey = fullPath;
+ useMutation: options => {
+ const mutationKey = [...fullPath];
if (options?.mutationKey) {
mutationKey.push(...(options.mutationKey as PropertyKey[]));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const mutationKey = fullPath; | |
| if (options?.mutationKey) { | |
| mutationKey.push(...(options.mutationKey as PropertyKey[])); | |
| const mutationKey = [...fullPath]; | |
| if (options?.mutationKey) { | |
| mutationKey.push(...(options.mutationKey as PropertyKey[])); | |
| } |
🤖 Prompt for AI Agents
packages/react-query-proxy/src/createProxy/createProxy.ts lines 54-56:
mutationKey is set by reference to fullPath then mutated with push, which
corrupts the shared fullPath array; change mutationKey to a copy of fullPath
(e.g., const mutationKey = [...fullPath] or const mutationKey =
fullPath.slice()) before pushing additional keys (preserve the existing
options.mutationKey casting/typing), or use concat to build a new array instead
of push so fullPath remains immutable.
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.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/react-query-proxy/src/createProxy/createProxy.ts (2)
4-4: Replaceanytypes with proper TypeScript types.The coding guidelines state: "Never use type
anyor cast to typeany. Always define the proper TypeScript types."This was already flagged in previous reviews.
Also applies to: 15-15, 32-32, 50-50, 61-61, 77-77, 80-80
As per coding guidelines.
54-56: Critical bug: mutationKey mutates the shared fullPath array.This was already flagged in previous reviews.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
packages/react-query-proxy/README.mdpackages/react-query-proxy/package.jsonpackages/react-query-proxy/src/createProxy/createProxy.spec.tspackages/react-query-proxy/src/createProxy/createProxy.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/react-query-proxy/package.json
- packages/react-query-proxy/src/createProxy/createProxy.spec.ts
- packages/react-query-proxy/README.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
packages/react-query-proxy/src/createProxy/createProxy.ts
🧠 Learnings (2)
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Never use type `any` or cast to type `any`. Always define the proper TypeScript types.
Applied to files:
packages/react-query-proxy/src/createProxy/createProxy.ts
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Never use deprecated methods from libraries.
Applied to files:
packages/react-query-proxy/src/createProxy/createProxy.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: provider-proxy-ci
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
| if (!proxyCache.has(object)) { | ||
| proxyCache.set(object, new Map()); | ||
| } |
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.
Critical type mismatch: Map vs Record.
Line 12 declares proxyCache as WeakMap<object, Record<string, any>>, but line 21 stores new Map(). The subsequent code (lines 25, 27, 28, 40, 65) accesses valueByPath using bracket notation (valueByPath[stringifiedPath]), which is appropriate for plain objects but incorrect for Maps.
Bracket notation on a Map instance sets properties on the Map object itself, not in its key-value storage. This breaks the caching mechanism.
🔎 Proposed fix
if (!proxyCache.has(object)) {
- proxyCache.set(object, new Map());
+ proxyCache.set(object, {});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!proxyCache.has(object)) { | |
| proxyCache.set(object, new Map()); | |
| } | |
| if (!proxyCache.has(object)) { | |
| proxyCache.set(object, {}); | |
| } |
🤖 Prompt for AI Agents
In packages/react-query-proxy/src/createProxy/createProxy.ts around lines 20-22
(and affecting lines 25, 27, 28, 40, 65), the code stores new Map() into
proxyCache but proxyCache is typed as WeakMap<object, Record<string, any>> and
later accesses use bracket notation; change the cache to consistently use Map by
updating the proxyCache declaration type to WeakMap<object, Map<string, any>>
and then replace all bracket-access uses (valueByPath[stringifiedPath]) with Map
methods (valueByPath.get(stringifiedPath), valueByPath.set(stringifiedPath,
value), valueByPath.has(stringifiedPath)) at the referenced lines so the Map’s
internal key-value store is used correctly.
| } | ||
|
|
||
| if (typeof value === "function") { | ||
| const key = `${stringifiedPath}.${prop as string}`; |
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.
Symbol property keys are not handled correctly.
Line 38 casts prop to string, but prop can be a symbol (since it's of type PropertyKey). If a function property has a symbol key, this would produce an incorrect cache key. Similarly, line 26's fullPath.join(".") would convert symbols to their string representation.
While symbols as SDK method names are uncommon, this could cause cache key collisions or incorrect behavior in edge cases.
Also applies to: 26-26
🤖 Prompt for AI Agents
In packages/react-query-proxy/src/createProxy/createProxy.ts around lines 26 and
38, PropertyKey values (which may be symbols) are being cast to string and
joined directly, causing incorrect cache keys for symbol-named properties;
replace the naive casts with a small serializer that converts keys with typeof
=== 'symbol' ? prop.toString() : String(prop) and use that serializer both when
building fullPath.join(".") and when creating the key at line 38 so symbols are
represented consistently and avoid collisions.
| const sdk = { | ||
| alerts: { | ||
| async list(input?: { page?: number }) { | ||
| return fetchAlerts(input) | ||
| }, | ||
|
|
||
| async create(input: { name: string }) { | ||
| return createAlert(input) | ||
| }, | ||
| }, | ||
| } |
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.
question: how would this be defined? with the auto generation code from this pr? https://github.com/akash-network/console/pull/2437/changes
Why
to have a way to automatically convert service into react-query observable hooks
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.