feat(server): Compression Plugin#869
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
dosubot
Bot
added
the
size:XL
|
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 CompressionPlugin implementations for Fetch and Node adapters (with tests), documents the plugin and updates docs navigation, re-exports the plugin from adapter barrels, adds compression devDependencies and an unbuild config disabling warnings-as-errors, and excludes the compression package from Dependabot dev-dependency updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FetchHandler as Fetch RPCHandler
participant CompPlugin as CompressionPlugin (Fetch)
participant UserInterceptors as User Interceptors
Client->>FetchHandler: HTTP Request
FetchHandler->>CompPlugin: Adapter interceptor (pre-user)
CompPlugin->>UserInterceptors: next()
UserInterceptors-->>CompPlugin: Response or NotMatched
alt Skip (not matched / already encoded / not compressible / no-transform / below threshold / filter denies)
CompPlugin-->>FetchHandler: Original Response
else Compressible & accepted encoding found
CompPlugin->>CompPlugin: Stream body through CompressionStream
CompPlugin-->>FetchHandler: Compressed Response (content-encoding set, content-length removed)
end
FetchHandler-->>Client: Response
sequenceDiagram
participant Client
participant NodeHandler as Node RPCHandler
participant CompMW as CompressionPlugin (Node)
participant UserInterceptors as User Interceptors
Client->>NodeHandler: HTTP Request
NodeHandler->>CompMW: Install interceptor/middleware (pre-user)
CompMW->>UserInterceptors: next()
UserInterceptors-->>CompMW: Write/End response
CompMW->>CompMW: Compression middleware wraps response streams
CompMW-->>NodeHandler: Restore original methods and finalize
NodeHandler-->>Client: (Possibly compressed) Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (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)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @unnoq, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a significant performance enhancement by adding a new Compression Plugin to the server. This plugin automatically compresses response bodies before they are sent to clients, which helps in reducing network bandwidth consumption and improving overall application speed. It is designed to work seamlessly with both Fetch and Node.js server adapters, offering flexible configuration options to tailor compression behavior. The changes include the core plugin implementation, dedicated test cases to ensure reliability, and updated documentation to guide users on its adoption.
Highlights
- New Compression Plugin: A new Compression Plugin has been introduced for the server, enabling automatic compression of response bodies to reduce bandwidth usage and improve performance.
- Dual Adapter Support: The plugin provides implementations for both Fetch and Node.js server adapters, ensuring broad compatibility across different server environments.
- Configurable Compression Options: The Compression Plugin is highly configurable, allowing users to specify preferred encoding schemes (e.g., gzip, deflate), set a minimum response size threshold for compression, and define a custom filter function for fine-grained control.
- Updated Documentation: Comprehensive documentation has been added, guiding users on how to import and set up the Compression Plugin within their oRPC handler configurations.
- Robust Test Coverage: New test suites have been added for both Fetch and Node.js adapters, validating the plugin's behavior under various conditions, including different
Accept-Encodingheaders, content types, and response sizes.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Code Review
This pull request introduces a new compression plugin for both Node.js and Fetch server adapters, complete with documentation and thorough test suites. The implementation is robust, leveraging the compression library for Node.js and the native CompressionStream API for Fetch environments. My feedback focuses on enhancing the clarity of the documentation and improving type safety within the Node.js adapter implementation.
More templates
@orpc/arktype
@orpc/client
@orpc/contract
@orpc/experimental-durable-event-iterator
@orpc/hey-api
@orpc/json-schema
@orpc/json-schema-typed
@orpc/nest
@orpc/openapi
@orpc/openapi-client
@orpc/otel
@orpc/react
@orpc/react-query
@orpc/server
@orpc/shared
@orpc/solid-query
@orpc/standard-server
@orpc/standard-server-aws-lambda
@orpc/standard-server-fetch
@orpc/standard-server-node
@orpc/standard-server-peer
@orpc/svelte-query
@orpc/tanstack-query
@orpc/trpc
@orpc/valibot
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
apps/content/docs/plugins/compression.md (1)
14-17: Avoid duplicate identifier in example; split imports per adapterA single TS snippet importing CompressionPlugin from two module paths defines the same identifier twice. Split into two examples (Node and Fetch) to prevent confusion.
Apply this edit:
-```ts -import { CompressionPlugin } from '@orpc/server/node' -import { CompressionPlugin } from '@orpc/server/fetch' -``` +For the Node.js adapter: +```ts +import { CompressionPlugin } from '@orpc/server/node' +``` + +For the Fetch adapter: +```ts +import { CompressionPlugin } from '@orpc/server/fetch' +```packages/server/src/adapters/node/compression-plugin.ts (1)
44-46: Type safety concern withas anycasts.
🧹 Nitpick comments (9)
packages/server/build.config.ts (1)
1-9: Consider documenting alternative solutions for ESM compatibility.While disabling
failOnWarnsolves the immediate issue with thecompressionpackage, this approach may hide other important warnings. Consider adding a comment about potential future solutions such as:
- Migrating to an ESM-compatible compression library when available
- Using dynamic imports with proper error handling
- Contributing ESM support to the upstream
compressionpackageexport default defineBuildConfig({ /** * Disable warnings as errors because we need to inline the `compression` package, * which is not ESModule-friendly. + * + * TODO: Consider migrating to an ESM-compatible compression library in the future + * to re-enable failOnWarn and maintain stricter build checks. */ failOnWarn: false, })packages/server/src/adapters/node/compression-plugin.ts (2)
6-7: Empty interface may confuse users.The empty interface extending
compression.CompressionOptionsdoesn't add value and may confuse users about its purpose. Consider either adding a comment explaining it's for future extensibility or using a type alias instead.-export interface CompressionPluginOptions extends compression.CompressionOptions { -} +/** + * Options for the compression plugin, extending the standard compression options. + * Reserved for future plugin-specific options. + */ +export interface CompressionPluginOptions extends compression.CompressionOptions { +}Or alternatively:
-export interface CompressionPluginOptions extends compression.CompressionOptions { -} +export type CompressionPluginOptions = compression.CompressionOptions
26-71: Consider adding timeout protection for compression operations.The compression operation could potentially hang or take excessive time for large payloads. Consider implementing a timeout mechanism to prevent indefinite blocking.
Would you like me to generate an implementation that includes timeout protection for the compression handler to ensure the server remains responsive even with problematic payloads?
packages/server/src/adapters/node/compression-plugin.test.ts (3)
88-112: Test may have race condition with error handling.The test manually sets status code and ends the response after the handler rejects, but there's a potential race condition if the handler partially writes to the response before throwing. Consider wrapping the response methods to ensure clean error handling.
it('should throw if rootInterceptor throws', async () => { const res = await request(async (req: IncomingMessage, res: ServerResponse) => { const handler = new RPCHandler(os.handler(() => output), { plugins: [ new CompressionPlugin(), ], rootInterceptors: [ async () => { throw new Error('Test error') }, ], }) - await expect(handler.handle(req, res)).rejects.toThrow('Test error') - - res.statusCode = 500 - res.end() + try { + await handler.handle(req, res) + } catch (error) { + if (!res.headersSent) { + res.statusCode = 500 + res.end() + } + expect(error).toEqual(new Error('Test error')) + } }) .post('/') .set('accept-encoding', 'gzip, deflate') .send({ input: 'test' }) expect(res.status).toBe(500) expect(res.headers['content-encoding']).toBeUndefined() })
114-137: Same race condition concern as the previous test.This test has the same potential race condition issue when handling errors.
Apply the same pattern as suggested for the previous test to ensure clean error handling:
- await expect(handler.handle(req, res)).rejects.toThrow('Test error') - - res.statusCode = 500 - res.end() + try { + await handler.handle(req, res) + } catch (error) { + if (!res.headersSent) { + res.statusCode = 500 + res.end() + } + expect(error).toEqual(new Error('Test error')) + }
7-28: Consider adding test for partial encoding support.The test assumes full encoding support when
accept-encodingincludes multiple values. Consider adding a test where the client only supports an encoding not prioritized by the server to ensure proper negotiation.+ it('should use the first supported encoding from client accept list', async () => { + const res = await request(async (req: IncomingMessage, res: ServerResponse) => { + const handler = new RPCHandler(os.handler(() => output), { + plugins: [ + new CompressionPlugin(), + ], + }) + + await handler.handle(req, res) + }) + .post('/') + .set('accept-encoding', 'br, deflate') // br not supported, should fall back to deflate + .send({ input: 'test' }) + + expect(res.status).toBe(200) + expect(res.headers['content-encoding']).toBe('deflate') + expect(res.body).toBeDefined() + })packages/server/src/adapters/fetch/compression-plugin.ts (1)
80-83: Consider validating content-length parsing.The current implementation assumes
Number(contentLength)will produce a valid number. Consider adding validation to handle malformed content-length headers.const contentLength = response.headers.get('content-length') - if (contentLength && Number(contentLength) < this.threshold) { - return result - } + if (contentLength) { + const length = Number(contentLength) + if (!isNaN(length) && length < this.threshold) { + return result + } + }packages/server/src/adapters/fetch/compression-plugin.test.ts (2)
276-314: Inconsistent response body handling in no-body test.The test sets
body: undefinedin the interceptor, but according to the Response specification, a 204 status automatically results in a null body regardless of what's provided. The test assertion correctly expectsresponse?.bodyto benull, notundefined.For clarity and to align with Response behavior, consider this improvement:
it('should not compress when response has no body', async () => { const handler = new RPCHandler(os.handler(() => 'output'), { strictGetMethodPluginEnabled: false, plugins: [ new CompressionPlugin(), ], interceptors: [ async (options) => { const result = await options.next() if (!result.matched) { return result } return { ...result, response: { ...result.response, status: 204, - body: undefined, + body: null, }, } }, ], }) const { response } = await handler.handle(new Request('https://example.com/', { method: 'POST', headers: { 'content-type': 'application/json', 'accept-encoding': 'gzip', }, body: JSON.stringify({}), })) expect(response?.body).toBe(null) expect(response?.headers.has('content-encoding')).toBe(false) expect(response?.status).toBe(204) })
495-519: Consider verifying filter invocation timing.The test correctly verifies that the custom filter prevents compression and is called with appropriate arguments. Good coverage of the filter functionality.
Consider adding a test to verify that the filter is only called when other conditions for compression are met (e.g., not called when content is already encoded or below threshold):
it('should not invoke filter when other compression conditions are not met', async () => { const filter = vi.fn(() => true) const handler = new RPCHandler(os.handler(() => 'small'), { plugins: [ new CompressionPlugin({ threshold: 1024, filter, }), ], }) const { response } = await handler.handle(new Request('https://example.com/', { method: 'POST', headers: { 'content-type': 'application/json', 'accept-encoding': 'gzip', }, body: JSON.stringify({}), })) expect(filter).not.toHaveBeenCalled() expect(response?.headers.has('content-encoding')).toBe(false) })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
.github/dependabot.yml(1 hunks)apps/content/.vitepress/config.ts(1 hunks)apps/content/docs/plugins/body-limit.md(1 hunks)apps/content/docs/plugins/compression.md(1 hunks)packages/server/build.config.ts(1 hunks)packages/server/package.json(1 hunks)packages/server/src/adapters/fetch/compression-plugin.test.ts(1 hunks)packages/server/src/adapters/fetch/compression-plugin.ts(1 hunks)packages/server/src/adapters/fetch/index.ts(1 hunks)packages/server/src/adapters/node/compression-plugin.test.ts(1 hunks)packages/server/src/adapters/node/compression-plugin.ts(1 hunks)packages/server/src/adapters/node/index.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
packages/server/src/adapters/node/compression-plugin.test.ts (3)
packages/server/src/builder.ts (2)
handler(273-280)os(336-352)packages/server/src/adapters/fetch/compression-plugin.ts (1)
CompressionPlugin(45-110)packages/server/src/adapters/node/compression-plugin.ts (1)
CompressionPlugin(13-73)
packages/server/src/adapters/fetch/compression-plugin.ts (4)
packages/server/src/adapters/node/compression-plugin.ts (2)
CompressionPluginOptions(6-7)CompressionPlugin(13-73)packages/server/src/context.ts (1)
Context(1-1)packages/server/src/adapters/fetch/plugin.ts (1)
FetchHandlerPlugin(6-8)packages/server/src/adapters/fetch/handler.ts (1)
FetchHandlerOptions(19-23)
packages/server/src/adapters/node/compression-plugin.ts (3)
packages/server/src/context.ts (1)
Context(1-1)packages/server/src/adapters/node/plugin.ts (1)
NodeHttpHandlerPlugin(6-8)packages/server/src/adapters/node/handler.ts (1)
NodeHttpHandlerOptions(20-24)
packages/server/src/adapters/fetch/compression-plugin.test.ts (2)
packages/server/src/builder.ts (2)
handler(273-280)os(336-352)packages/server/src/adapters/fetch/compression-plugin.ts (1)
CompressionPlugin(45-110)
🔇 Additional comments (10)
apps/content/.vitepress/config.ts (1)
144-144: Compression docs nav entry added: LGTMThe new Plugins entry points to the correct path and matches the new docs page.
.github/dependabot.yml (1)
25-25: Clarify intent: exclude-patterns only affects grouping, not updatesAdding "compression" under exclude-patterns keeps it out of the dev-dependencies-minor-patch group, but Dependabot will still open individual PRs for it. If your intent is to freeze minor/patch updates for compression (e.g., because it's inlined), add an ignore rule at this update config level.
If you want to ignore minor/patch updates for compression, add the following under the same npm update entry (sibling to groups):
ignore: - dependency-name: "compression" update-types: - "version-update:semver-minor" - "version-update:semver-patch"apps/content/docs/plugins/body-limit.md (1)
33-35: Nice addition: handler compatibility noteThe info admonition clarifies supported handlers and matches the pattern used in other plugin docs.
packages/server/package.json (1)
138-141: Verified npm packages exist at version 1.8.1
Bothcompressionand@types/compressionhave a published 1.8.1 release, so the^1.8.1ranges in packages/server/package.json (lines 138-141) are correct and won’t cause install failures.packages/server/src/adapters/node/index.ts (1)
1-5: LGTM! Clean module organization.The export follows the established pattern and maintains consistency with other plugin exports in the module.
packages/server/src/adapters/fetch/compression-plugin.ts (2)
96-106: Good implementation of streaming compression.The use of
pipeThroughwithCompressionStreamprovides efficient streaming compression without buffering the entire response. The header management is also correct - removingcontent-lengthand settingcontent-encoding.
115-121: Efficient content-type checking implementation.The regex-based approach for checking compressible content types is comprehensive and performs well. The function correctly handles null values.
packages/server/src/adapters/fetch/index.ts (1)
1-6: LGTM! Clean export addition.The compression plugin export is properly integrated with the existing barrel exports, maintaining consistent alphabetical ordering and following the established pattern.
packages/server/src/adapters/fetch/compression-plugin.test.ts (2)
353-397: Well-structured parameterized testing.Excellent use of
it.eachfor testing multiple content types. The test comprehensively covers various compressible MIME types and correctly verifies that content-length is removed after compression.
480-493: LGTM! Proper handling of non-matched routes.The test correctly verifies that the compression plugin doesn't interfere with non-matched routes, returning them unchanged with
matched: falseandresponse: undefined.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a Compression Plugin for both Fetch and Node.js adapters, adding response compression capabilities to reduce bandwidth usage and improve performance.
- Adds compression functionality for both Fetch (using CompressionStream) and Node.js (using the compression library) adapters
- Configurable options including compression encodings, size threshold, and custom filter functions
- Respects client Accept-Encoding headers and HTTP cache-control no-transform directives
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
packages/server/src/adapters/node/compression-plugin.ts |
Node.js compression plugin implementation using the compression library |
packages/server/src/adapters/fetch/compression-plugin.ts |
Fetch adapter compression plugin using CompressionStream API |
packages/server/src/adapters/node/compression-plugin.test.ts |
Test suite for Node.js compression plugin |
packages/server/src/adapters/fetch/compression-plugin.test.ts |
Comprehensive test suite for Fetch compression plugin |
packages/server/src/adapters/node/index.ts |
Exports the new Node.js compression plugin |
packages/server/src/adapters/fetch/index.ts |
Exports the new Fetch compression plugin |
packages/server/package.json |
Adds compression library dependencies |
packages/server/build.config.ts |
Disables warnings as errors for compression package inlining |
apps/content/docs/plugins/compression.md |
Documentation for the new compression plugin |
apps/content/docs/plugins/body-limit.md |
Updates to clarify handler compatibility |
apps/content/.vitepress/config.ts |
Adds compression plugin to documentation navigation |
.github/dependabot.yml |
Excludes compression package from automated updates |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores