feat(tanstack): migrate svelte query #366
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds Svelte framework support to the TanStack Query client: new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code graph analysis (3)packages/clients/tanstack-query/src/vue.ts (1)
packages/clients/tanstack-query/src/react.ts (1)
packages/clients/tanstack-query/src/svelte.ts (3)
⏰ 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). (2)
🔇 Additional comments (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude finished @ymc9's task —— View job Code Review Complete ✅Reviewing the Svelte Query migration PR. Here's my analysis plan:
Overall AssessmentThis PR successfully adds Svelte support to the TanStack Query client package. The implementation follows the established patterns from React and Vue implementations and maintains consistency with the existing codebase architecture. Code Quality & Best Practices ✅Strengths:
Observations:
Potential Issues & Suggestions1. Missing Error HandlingThe // svelte.ts:200-203
const modelDef = Object.values(schema.models).find((m) => m.name.toLowerCase() === model.toLowerCase());
if (!modelDef) {
throw new Error(`Model "${model}" not found in schema`);
}Suggestion: Consider more robust error messaging that lists available models. 2. Store Handling ConsistencyThe store unwrapping logic is well-implemented but could benefit from more explicit type guards: // svelte.ts:458-460
function isStore<T>(opt: unknown): opt is Readable<T> {
return typeof (opt as any)?.subscribe === 'function';
}This is correct but the 3. Context Key ExportThe context key is exported but could benefit from documentation: // svelte.ts:66
export const SvelteQueryContextKey = 'zenstack-svelte-query-context';Performance Considerations ✅Good practices implemented:
Security Concerns ✅No security issues identified:
Build Configuration ✅package.json changes are appropriate:
tsup.config.ts:
Test Coverage
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds Svelte support to the @zenstackhq/tanstack-query client package, enabling developers to use ZenStack with Svelte applications via TanStack Query. The implementation follows the existing patterns established for React and Vue integrations.
- Adds Svelte as a new framework target alongside React and Vue
- Implements comprehensive query and mutation hooks for Svelte using
@tanstack/svelte-query - Updates dependencies and build configuration to support the new Svelte entry point
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Adds Svelte 5.43.3 to the catalog for dependency management |
| pnpm-lock.yaml | Updates lock file with Svelte dependencies, decimal.js version bump, and TanStack Svelte Query integration |
| packages/clients/tanstack-query/tsup.config.ts | Adds Svelte entry point to build configuration |
| packages/clients/tanstack-query/src/svelte.ts | Implements complete Svelte query/mutation hooks API following React/Vue patterns |
| packages/clients/tanstack-query/package.json | Adds Svelte-related dependencies, exports, and peer dependencies |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Claude finished @ymc9's task —— View job Code Review Complete ✅Reviewing the Svelte Query migration PR. Here's my analysis plan:
Overall AssessmentThis PR successfully adds Svelte support to the TanStack Query client package. The implementation follows the established patterns from React and Vue implementations and maintains consistency with the existing codebase architecture. Code Quality & Best Practices ✅Strengths:
Observations:
Potential Issues & Suggestions1. Missing Error HandlingThe // svelte.ts:200-203
const modelDef = Object.values(schema.models).find((m) => m.name.toLowerCase() === model.toLowerCase());
if (!modelDef) {
throw new Error(`Model "${model}" not found in schema`);
}Suggestion: Consider more robust error messaging that lists available models. 2. Store Handling ConsistencyThe store unwrapping logic is well-implemented but could benefit from more explicit type guards: // svelte.ts:458-460
function isStore<T>(opt: unknown): opt is Readable<T> {
return typeof (opt as any)?.subscribe === 'function';
}This is correct but the 3. Context Key ExportThe context key is exported but could benefit from documentation: // svelte.ts:66
export const SvelteQueryContextKey = 'zenstack-svelte-query-context';Performance Considerations ✅Good practices implemented:
Security Concerns ✅No security issues identified:
Build Configuration ✅package.json changes are appropriate:
tsup.config.ts:
Test Coverage
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
packages/clients/tanstack-query/package.json(3 hunks)packages/clients/tanstack-query/src/svelte.ts(1 hunks)packages/clients/tanstack-query/tsup.config.ts(1 hunks)pnpm-workspace.yaml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/clients/tanstack-query/src/svelte.ts (4)
packages/clients/tanstack-query/src/utils/common.ts (8)
APIContext(111-126)ExtraQueryOptions(101-106)ExtraMutationOptions(81-96)getQueryKey(180-190)fetcher(128-159)marshal(192-199)setupInvalidation(234-255)setupOptimisticUpdate(313-357)packages/schema/src/schema.ts (2)
SchemaDef(10-18)GetModels(109-109)packages/orm/src/client/crud-types.ts (20)
FindUniqueArgs(669-671)SelectSubset(509-515)ModelResult(135-179)FindArgs(648-664)CreateArgs(677-682)CreateManyArgs(684-684)BatchResult(201-201)CreateManyAndReturnArgs(686-692)UpdateArgs(815-821)UpdateManyArgs(823-826)UpdateManyAndReturnArgs(828-834)UpsertArgs(842-849)DeleteArgs(961-966)DeleteManyArgs(968-971)CountArgs(977-982)CountResult(988-998)AggregateArgs(1004-1018)AggregateResult(1044-1072)GroupByArgs(1091-1108)GroupByResult(1110-1144)packages/common-helpers/src/lower-case-first.ts (1)
lowerCaseFirst(1-3)
⏰ 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). (4)
- GitHub Check: Upload results
- GitHub Check: claude-review
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: build-test (20.x, postgresql)
|
Claude finished @ymc9's task —— View job Code Review Complete ✅Reviewing the Svelte Query migration PR. Here's my analysis plan:
Overall AssessmentThis PR successfully adds Svelte support to the TanStack Query client package. The implementation follows the established patterns from React and Vue implementations and maintains consistency with the existing codebase architecture. Code Quality & Best Practices ✅Strengths:
Observations:
Potential Issues & Suggestions1. Missing Error HandlingThe // svelte.ts:200-203
const modelDef = Object.values(schema.models).find((m) => m.name.toLowerCase() === model.toLowerCase());
if (!modelDef) {
throw new Error(`Model "${model}" not found in schema`);
}Suggestion: Consider more robust error messaging that lists available models. 2. Store Handling ConsistencyThe store unwrapping logic is well-implemented but could benefit from more explicit type guards: // svelte.ts:458-460
function isStore<T>(opt: unknown): opt is Readable<T> {
return typeof (opt as any)?.subscribe === 'function';
}This is correct but the 3. Context Key ExportThe context key is exported but could benefit from documentation: // svelte.ts:66
export const SvelteQueryContextKey = 'zenstack-svelte-query-context';Performance Considerations ✅Good practices implemented:
Security Concerns ✅No security issues identified:
Build Configuration ✅package.json changes are appropriate:
tsup.config.ts:
Test Coverage
|
Summary by CodeRabbit
New Features
Refactor
Chores