Feat/compt 40 create query factory#3
Conversation
- Add createQuery(keyFn, fetcher) returning QueryDefinition<TParams, TData> - TData and TParams fully inferred from fetcher signature, zero manual annotation - queryKey returns stable readonly tuple via keyFn - useQuery shorthand hook wraps useTanstackQuery with typed params - Export from src/index.ts - Add @tanstack/react-query as peerDependency (>=5) and devDependency - Add pnpm cssstyle override to fix Node.js v22 + jsdom@28 ESM compat issue - Fix duplicate import in vitest.config.ts - 9 tests, 100% coverage on createQuery.ts, 88.6% overall Closes COMPT-40
There was a problem hiding this comment.
Pull request overview
This PR introduces a small “query factory” abstraction over TanStack React Query v5 and exposes it through the library’s public API (src/index.ts), along with adding the needed dependency metadata.
Changes:
- Added
createQueryfactory that returns{ queryKey, queryFn, useQuery }for a given key function + fetcher. - Added a Vitest test suite for
createQuery. - Exported the new query module from the package entrypoint and added
@tanstack/react-queryas a peer/dev dependency (plus an npm override).
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Removes a duplicate import. |
| src/query/index.ts | Adds a barrel export for the query module. |
| src/query/createQuery.ts | Implements the createQuery factory and types. |
| src/query/createQuery.test.tsx | Adds unit/integration tests for the factory and hook behavior. |
| src/index.ts | Exposes the query module in the public API. |
| package.json | Adds react-query peer/dev dependency and a cssstyle override. |
| package-lock.json | Updates lockfile to reflect dependency/override changes. |
| it('queryKey produces a stable reference for the same params', () => { | ||
| const def = createQuery( | ||
| (id: number) => ['item', id] as const, | ||
| async (id: number) => id, | ||
| ); | ||
|
|
||
| const first = def.queryKey(1); | ||
| const second = def.queryKey(1); | ||
| expect(first).toEqual(second); | ||
| }); |
There was a problem hiding this comment.
The test name claims the query key reference is stable for the same params, but the assertion only checks deep equality (toEqual). Also, keyFn as written returns a new array each call, so a true referential-stability assertion (toBe) would fail. Consider either updating the assertion/test to reflect referential stability (and implementing memoization), or renaming the test to only assert structural equality.
src/query/createQuery.ts
Outdated
| queryKey: keyFn, | ||
| queryFn: fetcher, | ||
| useQuery(params, options) { | ||
|
|
There was a problem hiding this comment.
There is stray whitespace on the blank line inside useQuery (will typically fail Prettier/ESLint no-trailing-spaces rules). Remove the extra spaces so formatting stays clean.
Summary
Why
Checklist
npm run lintpassesnpm run typecheckpassesnpm testpassesnpm run buildpassesnpx changeset) if this affects consumersNotes