Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Oct 30, 2025

Summary by CodeRabbit

  • New Features

    • Added SvelteKit adapter integration for server-side request handling, including support for configurable route prefixes and client resolution.
  • Tests

    • Added comprehensive test suite for SvelteKit adapter covering RPC and REST operations.

Copilot AI review requested due to automatic review settings October 30, 2025 00:57
@coderabbitai
Copy link

coderabbitai bot commented Oct 30, 2025

Walkthrough

This pull request introduces a new SvelteKit adapter for the server package, applies consistent naming conventions to adapter options types across multiple adapters, replaces wildcard re-exports with explicit named exports in adapter index files, and updates the build and package configuration to support the new integration.

Changes

Cohort / File(s) Summary
SvelteKit adapter implementation
packages/server/src/adapter/sveltekit/handler.ts, packages/server/src/adapter/sveltekit/index.ts
Introduces new SvelteKit request handler with SvelteKitHandlerOptions interface and createHandler factory function. Exports SvelteKitHandler and SvelteKitHandlerOptions from index.
Naming consistency for Express adapter
packages/server/src/adapter/express/middleware.ts, packages/server/src/adapter/express/index.ts
Renames MiddlewareOptions to ExpressMiddlewareOptions interface and updates corresponding exports.
Naming consistency for Fastify adapter
packages/server/src/adapter/fastify/plugin.ts, packages/server/src/adapter/fastify/index.ts
Renames PluginOptions to FastifyPluginOptions interface and updates plugin handler signature and index exports.
Naming consistency for Nuxt adapter
packages/server/src/adapter/nuxt/handler.ts
Renames HandlerOptions to NuxtHandlerOptions interface.
Export consolidation
packages/server/src/adapter/elysia/index.ts, packages/server/src/adapter/hono/index.ts, packages/server/src/adapter/nuxt/index.ts
Changes from wildcard re-exports to explicit named exports (createElysiaHandler/ElysiaOptions, createHonoHandler/HonoOptions, createEventHandler/NuxtHandlerOptions).
Build and package configuration
packages/server/package.json, packages/server/tsup.config.ts
Adds SvelteKit export to package.json with peer and dev dependencies. Adds sveltekit build entry to tsup configuration.
SvelteKit adapter tests
packages/server/test/adapter/sveltekit.test.ts
Comprehensive test suite covering RPC and REST handler operations with HTTP status validation and data integrity checks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • SvelteKit handler implementation: New adapter code with request parsing, client resolution, and error handling logic requires careful review of correctness and consistency with existing adapters
  • Naming refactoring pattern: Multiple adapters follow identical type-renaming pattern, reducing cognitive load but each file needs verification for consistency
  • Export changes: Straightforward conversion from wildcard to explicit exports, but verify no unintended symbols are hidden
  • Test coverage: New test file is comprehensive but should be validated against actual SvelteKit request event handling

Possibly related PRs

Poem

🐰 A SvelteKit friend hops into the fold,
With handlers named true and types renamed bold,
From wildcards tamed to exports so clear,
The rabbit cheers—consistency thrives here! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "feat(server): migrate sveltekit adapter" accurately describes the primary objective of the changeset. The pull request introduces a completely new SvelteKit adapter with handler implementation, index exports, comprehensive tests, and build configuration entry. While the changeset also includes supporting refactoring across other adapters (renaming options types to be adapter-specific and normalizing export patterns in Express, Fastify, Elysia, Hono, and Nuxt adapters), these appear to be preparatory changes to establish a consistent API pattern. The title is specific, clear, and allows a teammate reviewing history to understand the main feature being delivered.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/sveltekit

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link

claude bot commented Oct 30, 2025

Claude finished @ymc9's task —— View job


Code Review: SvelteKit Adapter Migration

I've reviewed the SvelteKit adapter implementation and compared it with existing adapter patterns. Here's my comprehensive feedback:

Review Tasks:

  • Read and analyze new SvelteKit adapter implementation
  • Review test coverage and quality
  • Compare with existing adapter patterns for consistency
  • Check package.json dependencies and configuration
  • Identify potential bugs, security issues, or performance concerns
  • Provide constructive feedback and recommendations

Positive Aspects

  1. Consistent Architecture: The SvelteKit adapter follows the established pattern used by other adapters (Nuxt, Hono, etc.) with proper separation of concerns.

  2. Comprehensive Test Coverage: The test file covers both RPC and REST API handlers with extensive CRUD operations, which is excellent.

  3. Proper TypeScript Support: Good use of generics with Schema extends SchemaDef and proper typing throughout.

  4. Error Handling: Implements proper error handling with internal error logging and user-friendly error responses.

  5. Package Configuration: Correctly updated package.json with exports, dependencies, and build configuration.


🔍 Issues and Recommendations

Critical Issues

  1. Potential JSON Parsing Vulnerability (packages/server/src/adapter/sveltekit/handler.ts:42)

    if (text) {
        requestBody = JSON.parse(text); // ⚠️ No try-catch for malformed JSON
    }

    Risk: Malformed JSON will crash the application
    Fix: Wrap in try-catch and return 400 for invalid JSON

  2. Request Body Handling Issue (packages/server/src/adapter/sveltekit/handler.ts:39-44)

    if (event.request.body) {
        const text = await event.request.text();

    Issue: This consumes the request stream. If called elsewhere, it will fail.
    Recommendation: Check content-type header and method before reading body.

Security Concerns

  1. Missing Input Validation

    • No validation of the prefix parameter
    • No sanitization of path parameters
    • Consider adding input validation for security
  2. Error Information Leakage

    • Internal errors are logged but generic message returned (good)
    • Consider rate limiting for failed requests

Performance Considerations

  1. Unnecessary String Operations (packages/server/src/adapter/sveltekit/handler.ts:37,46)
    const query = Object.fromEntries(event.url.searchParams); // Could be optimized
    const path = event.url.pathname.substring(options.prefix.length);
    Minor: These are fine for most use cases, but could be optimized for high-traffic scenarios.

Code Quality Issues

  1. Inconsistent Error Responses

    • Client error (400) vs internal error (500) responses have different structures
    • Consider standardizing error response format
  2. Missing JSDoc Documentation

    • The createHandler function lacks detailed JSDoc comments
    • Other adapters have better documentation

Test Issues

  1. Mock Request Implementation (packages/server/test/adapter/sveltekit.test.ts:138-149)
    resolve: async () => {
        throw new Error('should not be called'); // Could be more descriptive
    }
    Minor: The mock could be more realistic for better test coverage.

🔧 Recommended Fixes

  1. Add JSON parsing safety:

    if (text) {
        try {
            requestBody = JSON.parse(text);
        } catch (error) {
            return new Response(JSON.stringify({ message: 'Invalid JSON in request body' }), {
                status: 400,
                headers: { 'content-type': 'application/json' }
            });
        }
    }
  2. Improve request body handling:

    if (event.request.body && (event.request.method === 'POST' || event.request.method === 'PUT' || event.request.method === 'PATCH')) {
        const contentType = event.request.headers.get('content-type');
        if (contentType?.includes('application/json')) {
            // Then read and parse JSON
        }
    }
  3. Add input validation:

    if (!options.prefix || !options.prefix.startsWith('/')) {
        throw new Error('prefix must be a valid path starting with "/"');
    }

📊 Overall Assessment

Rating: Good (B+)

This is a solid implementation that maintains consistency with the existing codebase architecture. The test coverage is comprehensive, and the code follows established patterns. The main concerns are around error handling robustness and security considerations, which are easily addressable.

Recommendation: ✅ Approve with minor fixes - Address the JSON parsing vulnerability and consider the other suggestions for production readiness.


Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds SvelteKit adapter support to the ZenStack server package, enabling CRUD request handling for SvelteKit applications. The changes also standardize adapter option type naming across all framework adapters for consistency.

  • Adds a new SvelteKit adapter with handler implementation
  • Renames adapter option types to follow a consistent naming pattern (e.g., MiddlewareOptionsExpressMiddlewareOptions)
  • Updates package exports to use explicit named exports instead of wildcard exports

Reviewed Changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pnpm-lock.yaml Adds @sveltejs/kit and its dependencies to support the new SvelteKit adapter
packages/server/package.json Adds SvelteKit as an optional peer dependency and exports the new adapter entry point
packages/server/tsup.config.ts Adds build configuration for the sveltekit adapter bundle
packages/server/src/adapter/sveltekit/index.ts Exports the SvelteKit handler and options type
packages/server/src/adapter/sveltekit/handler.ts Implements the SvelteKit request handler as a SvelteKit hooks handler
packages/server/test/adapter/sveltekit.test.ts Adds comprehensive tests for both RPC and REST API handlers
packages/server/src/adapter/nuxt/index.ts Changes from wildcard export to explicit named exports
packages/server/src/adapter/nuxt/handler.ts Renames HandlerOptions to NuxtHandlerOptions
packages/server/src/adapter/hono/index.ts Changes from wildcard export to explicit named exports
packages/server/src/adapter/fastify/plugin.ts Renames PluginOptions to FastifyPluginOptions
packages/server/src/adapter/fastify/index.ts Updates export to use renamed FastifyPluginOptions
packages/server/src/adapter/express/middleware.ts Renames MiddlewareOptions to ExpressMiddlewareOptions
packages/server/src/adapter/express/index.ts Updates export to use renamed ExpressMiddlewareOptions
packages/server/src/adapter/elysia/index.ts Changes from wildcard export to explicit named exports
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (1)
packages/server/test/adapter/sveltekit.test.ts (1)

138-154: Helper functions look good, with one optional note.

The makeRequest and unmarshal helpers properly simulate SvelteKit's request/response structure.

Minor observation: The useSuperJson parameter in unmarshal (line 151) defaults to false and is never called with true in the current tests. This may be intentional for future use, but if SuperJSON serialization testing isn't planned, the parameter could be removed.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between da048dd and bba4ab7.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (13)
  • packages/server/package.json (4 hunks)
  • packages/server/src/adapter/elysia/index.ts (1 hunks)
  • packages/server/src/adapter/express/index.ts (1 hunks)
  • packages/server/src/adapter/express/middleware.ts (2 hunks)
  • packages/server/src/adapter/fastify/index.ts (1 hunks)
  • packages/server/src/adapter/fastify/plugin.ts (2 hunks)
  • packages/server/src/adapter/hono/index.ts (1 hunks)
  • packages/server/src/adapter/nuxt/handler.ts (1 hunks)
  • packages/server/src/adapter/nuxt/index.ts (1 hunks)
  • packages/server/src/adapter/sveltekit/handler.ts (1 hunks)
  • packages/server/src/adapter/sveltekit/index.ts (1 hunks)
  • packages/server/test/adapter/sveltekit.test.ts (1 hunks)
  • packages/server/tsup.config.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
{packages,samples,tests}/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place packages only under packages/, samples/, or tests/

Files:

  • packages/server/src/adapter/express/index.ts
  • packages/server/src/adapter/fastify/plugin.ts
  • packages/server/tsup.config.ts
  • packages/server/src/adapter/express/middleware.ts
  • packages/server/src/adapter/nuxt/handler.ts
  • packages/server/package.json
  • packages/server/src/adapter/sveltekit/handler.ts
  • packages/server/src/adapter/elysia/index.ts
  • packages/server/src/adapter/sveltekit/index.ts
  • packages/server/src/adapter/hono/index.ts
  • packages/server/src/adapter/fastify/index.ts
  • packages/server/src/adapter/nuxt/index.ts
  • packages/server/test/adapter/sveltekit.test.ts
🧬 Code graph analysis (5)
packages/server/src/adapter/fastify/plugin.ts (2)
packages/sdk/src/schema/schema.ts (1)
  • SchemaDef (10-18)
packages/server/src/adapter/common.ts (1)
  • CommonAdapterOptions (8-13)
packages/server/src/adapter/express/middleware.ts (2)
packages/sdk/src/schema/schema.ts (1)
  • SchemaDef (10-18)
packages/server/src/adapter/common.ts (1)
  • CommonAdapterOptions (8-13)
packages/server/src/adapter/nuxt/handler.ts (3)
packages/sdk/src/schema/schema.ts (1)
  • SchemaDef (10-18)
packages/server/src/adapter/common.ts (1)
  • CommonAdapterOptions (8-13)
packages/orm/src/client/contract.ts (1)
  • ClientContract (50-170)
packages/server/src/adapter/sveltekit/handler.ts (4)
packages/sdk/src/schema/schema.ts (1)
  • SchemaDef (10-18)
packages/server/src/adapter/common.ts (2)
  • CommonAdapterOptions (8-13)
  • logInternalError (15-17)
packages/orm/src/client/contract.ts (1)
  • ClientContract (50-170)
packages/server/src/types.ts (1)
  • Response (52-62)
packages/server/test/adapter/sveltekit.test.ts (2)
packages/testtools/src/client.ts (1)
  • createTestClient (52-169)
packages/server/test/utils.ts (1)
  • makeUrl (31-33)
⏰ 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). (3)
  • GitHub Check: Upload results
  • GitHub Check: build-test (20.x, postgresql)
  • GitHub Check: build-test (20.x, sqlite)
🔇 Additional comments (20)
packages/server/package.json (1)

102-111: LGTM! SvelteKit integration follows established adapter patterns.

The export structure, dependency versions, and optional peer dependency configuration are consistent with the existing adapters in this package.

Also applies to: 124-124, 145-145, 177-179

packages/server/src/adapter/elysia/index.ts (1)

1-1: LGTM! Explicit exports improve API surface control.

Replacing the wildcard export with explicit named exports is a good practice that prevents unintended public API exposure.

packages/server/src/adapter/hono/index.ts (1)

1-1: LGTM! Consistent with the adapter API surface narrowing pattern.

The explicit export aligns with the refactoring across other adapters in this PR.

packages/server/src/adapter/sveltekit/handler.ts (3)

9-19: LGTM! Interface definition follows adapter patterns.

The SvelteKitHandlerOptions interface correctly extends CommonAdapterOptions and defines the required prefix and getClient properties for SvelteKit integration.


24-76: LGTM! Handler implementation follows SvelteKit patterns.

The handler correctly:

  • Checks for prefix-matching routes
  • Retrieves the client
  • Parses query and body
  • Delegates to apiHandler
  • Returns proper Response objects
  • Falls through to resolve(event) for non-API routes

78-78: LGTM! Export alias improves discoverability.

Providing SvelteKitHandler as an alias for createHandler makes the API more intuitive and consistent with naming conventions.

packages/server/src/adapter/sveltekit/index.ts (1)

1-1: LGTM! Exports align with adapter conventions.

The explicit exports of SvelteKitHandler and SvelteKitHandlerOptions are consistent with the patterns used in other adapters (Nuxt, Hono, etc.).

packages/server/tsup.config.ts (1)

12-12: LGTM! Build entry correctly configured.

The sveltekit entry is properly added to the build configuration, enabling the adapter to be bundled and exported.

packages/server/src/adapter/express/index.ts (1)

1-1: LGTM! Naming consistency improvement.

Renaming to ExpressMiddlewareOptions makes the type more specific and aligns with the naming conventions used across other adapters in this PR.

packages/server/src/adapter/nuxt/index.ts (1)

1-1: LGTM! Explicit exports improve API clarity.

The change from wildcard export to explicit named exports is consistent with the refactoring pattern applied across all adapters in this PR.

packages/server/src/adapter/fastify/index.ts (1)

1-1: LGTM! Consistent naming pattern.

The type rename to FastifyPluginOptions improves clarity and follows the adapter-specific naming convention used across other adapters in this PR.

packages/server/test/adapter/sveltekit.test.ts (3)

1-7: LGTM! Imports are well-organized.

All necessary dependencies are properly imported for testing the SvelteKit adapter with both RPC and REST handlers.


8-79: LGTM! Comprehensive RPC handler test coverage.

The test suite thoroughly validates the RPC handler with:

  • CRUD operations (create, findMany, update, delete)
  • Nested relational writes
  • Filtering and querying
  • Aggregations (count, aggregate, groupBy)

All assertions properly verify status codes and response data structure.


81-136: LGTM! REST handler test coverage is solid.

The test suite validates REST API operations with:

  • Proper REST format (type/attributes structure)
  • Query parameter filtering
  • Full CRUD cycle
  • Direct client verification of final state (line 134)
packages/server/src/adapter/express/middleware.ts (2)

9-9: LGTM! Type rename improves consistency.

Renaming to ExpressMiddlewareOptions aligns with the adapter-specific naming convention applied across all adapters in this PR (Fastify, Nuxt, SvelteKit).


30-30: LGTM! Function signature updated correctly.

The factory function signature properly uses the renamed ExpressMiddlewareOptions type, maintaining type safety.

packages/server/src/adapter/fastify/plugin.ts (2)

10-10: LGTM! Type rename improves consistency.

Renaming to FastifyPluginOptions follows the adapter-specific naming convention and aligns with the export in fastify/index.ts.


26-26: LGTM! Plugin handler signature updated correctly.

The plugin callback properly uses the renamed FastifyPluginOptions<SchemaDef> type, maintaining type safety.

packages/server/src/adapter/nuxt/handler.ts (2)

17-17: LGTM! Type rename improves consistency.

Renaming to NuxtHandlerOptions aligns with the adapter-specific naming convention used across all adapters in this PR.


24-24: LGTM! Function signature updated correctly.

The createEventHandler function properly uses the renamed NuxtHandlerOptions type, maintaining type safety.

@ymc9 ymc9 merged commit fe5f61d into dev Oct 30, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants