-
-
Notifications
You must be signed in to change notification settings - Fork 40
feat: add TypeBox support #454
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis set of changes introduces support for the TypeBox schema library within the oRPC framework. It expands the accepted schema types in the contract package to include TypeBox schemas, adds a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Contract
participant TypeBoxConverter
participant OpenAPIGenerator
User->>Contract: Define contract with TypeBox schema
Contract->>TypeBoxConverter: Pass TypeBox schema for conversion
TypeBoxConverter->>TypeBoxConverter: Check if schema is TypeBox (Kind property)
TypeBoxConverter-->>Contract: Return JSON Schema (direct from TypeBox)
Contract->>OpenAPIGenerator: Provide JSON Schema
OpenAPIGenerator-->>User: Generate OpenAPI spec
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 1
🔭 Outside diff range comments (1)
packages/typebox/src/converter.ts (1)
1-22
: 💡 Verification agent🧩 Analysis chain
Verify the implementation with different TypeBox schema types
The converter assumes all TypeBox schemas are directly usable as JSON Schema without transformation. Verify this works correctly with various TypeBox schema types.
🏁 Script executed:
#!/bin/bash # Look for TypeBox schema usage in tests to verify compatibility rg -A 3 "Kind\s+in\s+schema" --type tsLength of output: 399
Add comprehensive tests for TypeBox schema conversion
The converter currently assumes every TypeBox schema can be spread directly into a JSON Schema. To gain confidence that this holds for all schema constructs, please add unit tests covering:
- Primitive types:
Type.String()
,Type.Number()
,Type.Boolean()
,Type.Null()
- Complex types:
Type.Array(Type.String())
,Type.Object({ … })
- Composite types:
Type.Union([ … ])
,Type.Intersect([ … ])
,Type.Tuple([ … ])
- Other constructs:
Type.Literal('foo')
,Type.Enum(...)
,Type.Dict(...)
, etc.These tests should instantiate each schema via the converter and verify the output matches the expected JSON Schema (e.g. by JSON-stringifying and comparing). This will ensure no special handling is required for any TypeBox feature.
🧰 Tools
🪛 Biome (1.9.4)
[error] 6-6: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 9-11: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
🧹 Nitpick comments (7)
packages/typebox/.gitignore (1)
16-23
: Add OS and editor-specific ignores
Consider extending ignore patterns to include OS artifacts (.DS_Store
,Thumbs.db
) and common editor/IDE dirs (e.g.,.vscode/
,.idea/
) to reduce accidental commits of such files.packages/typebox/src/converter.test.ts (1)
1-28
: Missing tests for more complex TypeBox schemasWhile the current tests cover basic TypeBox schemas, consider adding tests for more complex schema features like arrays, unions, intersections, and nested objects to ensure comprehensive coverage.
Example additional test cases:
it('typeBoxToJsonSchemaConverter handles complex schemas', async () => { const converter = new TypeBoxToJsonSchemaConverter() // Test array expect(converter.convert(Type.Array(Type.String()), { strategy: 'input' })) .toMatchObject([true, { type: 'array', items: { type: 'string' } }]) // Test union expect(converter.convert(Type.Union([Type.String(), Type.Number()]), { strategy: 'input' })) .toMatchObject([true, { oneOf: [{ type: 'string' }, { type: 'number' }] }]) // Test nested object expect(converter.convert( Type.Object({ user: Type.Object({ name: Type.String(), age: Type.Number() }) }), { strategy: 'input' } )).toMatchObject([ true, { type: 'object', properties: { user: { type: 'object', properties: { name: { type: 'string' }, age: { type: 'number' } }, required: ['name', 'age'] } }, required: ['user'] } ]) })packages/typebox/README.md (2)
97-100
: Add alt text to the sponsors imageFor better accessibility, add alternative text to the sponsors image.
- <a href="https://cdn.jsdelivr.net/gh/unnoq/unnoq/sponsors.svg"> - <img src='https://cdn.jsdelivr.net/gh/unnoq/unnoq/sponsors.svg'/> + <a href="https://cdn.jsdelivr.net/gh/unnoq/unnoq/sponsors.svg"> + <img src='https://cdn.jsdelivr.net/gh/unnoq/unnoq/sponsors.svg' alt="oRPC Sponsors"/>🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
99-99: Images should have alternate text (alt text)
null(MD045, no-alt-text)
70-93
: Inconsistency between code example and documentationThe example uses
experimental_TypeBoxToJsonSchemaConverter
while the documentation text on line 66 refers to it as justTypeBoxToJsonSchemaConverter
. Ensure consistency in naming.Either update line 66 to include the
experimental_
prefix or update the import in line 72 to use the non-prefixed name, depending on the actual exported name.packages/typebox/src/converter.ts (3)
5-6
: Consider using a type alias for empty interfaceAn empty interface is equivalent to
{}
. Using a type alias would be more appropriate here.-export interface experimental_TypeBoxToJsonSchemaConverterOptions {} +export type experimental_TypeBoxToJsonSchemaConverterOptions = {}🧰 Tools
🪛 Biome (1.9.4)
[error] 6-6: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
8-11
: Simplify constructor implementationThe constructor doesn't perform any operations but serves as a placeholder for future options.
- constructor(_options: experimental_TypeBoxToJsonSchemaConverterOptions = {}) { - // No options currently needed - } + constructor(_options: experimental_TypeBoxToJsonSchemaConverterOptions = {}) {}🧰 Tools
🪛 Biome (1.9.4)
[error] 9-11: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
17-20
: Type assertion could be improvedThe
convert
method uses a type assertion withas any
. Consider using a more specific type.- return [true, { ...schema } as any] + return [true, { ...schema } as Exclude<JSONSchema, boolean>]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
packages/contract/package.json
(1 hunks)packages/contract/src/schema.ts
(1 hunks)packages/typebox/.gitignore
(1 hunks)packages/typebox/README.md
(1 hunks)packages/typebox/package.json
(1 hunks)packages/typebox/src/converter.test.ts
(1 hunks)packages/typebox/src/converter.ts
(1 hunks)packages/typebox/src/index.ts
(1 hunks)packages/typebox/tsconfig.json
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/typebox/src/converter.ts (3)
packages/openapi/src/schema-converter.ts (2)
ConditionalSchemaConverter
(13-15)SchemaConvertOptions
(5-7)packages/contract/src/schema.ts (1)
AnySchema
(8-8)packages/openapi/src/schema.ts (1)
JSONSchema
(6-6)
🪛 LanguageTool
packages/typebox/README.md
[misspelling] ~33-~33: Possible spelling mistakes found.
Context: ...tack Query (React, Vue, Solid, Svelte), Pinia Colada, and more. - 🚀 Server Actions: Ful...
(EN_MULTITOKEN_SPELLING_TWO)
[style] ~51-~51: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... your API or implement API contract. - [@orpc/client](https://www.npmjs.com/package/@...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~52-~52: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... API on the client with type-safety. - [@orpc/react](https://www.npmjs.com/package/@o...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~53-~53: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...with React and React Server Actions. - [@orpc/react-query](https://www.npmjs.com/pack...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~54-~54: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...test/docs/framework/react/overview). - [@orpc/vue-query](https://www.npmjs.com/packag...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~55-~55: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...latest/docs/framework/vue/overview). - [@orpc/solid-query](https://www.npmjs.com/pack...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~56-~56: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...test/docs/framework/solid/overview). - [@orpc/svelte-query](https://www.npmjs.com/pac...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~57-~57: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...est/docs/framework/svelte/overview). - [@orpc/vue-colada](https://www.npmjs.com/packa...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[misspelling] ~57-~57: Possible spelling mistakes found.
Context: ...ge/@orpc/vue-colada): Integration with Pinia Colada. - [@or...
(EN_MULTITOKEN_SPELLING_TWO)
[style] ~58-~58: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ada](https://pinia-colada.esm.dev/). - [@orpc/openapi](https://www.npmjs.com/package/...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~59-~59: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...I specs and handle OpenAPI requests. - [@orpc/zod](https://www.npmjs.com/package/@orp...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~60-~60: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...tps://zod.dev/) doesn't support yet. - [@orpc/valibot](https://www.npmjs.com/package/...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~61-~61: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...rom Valibot. - [@orpc/arktype](https://www.npmjs.com/package/...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~62-~62: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...from ArkType. - [@orpc/typebox](https://www.npmjs.com/package/...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.17.2)
packages/typebox/README.md
99-99: Images should have alternate text (alt text)
null
(MD045, no-alt-text)
🪛 Biome (1.9.4)
packages/typebox/src/converter.ts
[error] 6-6: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 9-11: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
🔇 Additional comments (12)
packages/typebox/.gitignore (3)
1-5
: Verify dotfile ignore rules
Ensure that ignoring all hidden files (.*
) except.gitignore
and.*.example
matches project requirements. Consider whether user-specific dotfiles like.npmrc
,.env
, or other environment files should be explicitly included or excluded here.
6-15
: Generated directories are properly ignored
Patterns fornode_modules/
, build outputs (out/
,dist/
,build/
), logs (logs/
), coverage, and temp folders comprehensively cover typical artifacts in this package.
24-26
: Manual ignore patterns look good
Ignoring*.local
and*.pem
covers local config and key files appropriately.packages/contract/package.json (1)
42-42
: Added TypeBox dependency - LGTMThe addition of @sinclair/typebox as a development dependency with version ^0.30.0 is appropriate for enabling TypeBox schema support in the contract package.
packages/typebox/src/index.ts (1)
1-1
: Clean export from converter moduleThe export pattern follows standard practices for TypeScript package organization, making the converter implementation accessible through the package's main entry point.
packages/contract/src/schema.ts (1)
2-2
: Appropriate TypeBox importThe import of TSchema from @sinclair/typebox is correctly added to support TypeBox integration.
packages/typebox/tsconfig.json (1)
1-20
: Appropriate TypeScript configurationThe TypeScript configuration correctly:
- Extends the library base configuration
- Sets the source directory
- References dependent packages
- Includes/excludes appropriate files
This follows the project's conventions for package configuration.
packages/typebox/src/converter.test.ts (2)
5-20
: Well-structured tests for the convert methodThe tests for the convert method comprehensively verify that TypeBox schemas are correctly converted to JSON Schema format, testing both simple string types and more complex object types with required properties.
22-28
: Good condition method testsThese tests properly verify that the converter correctly identifies TypeBox schemas and distinguishes them from other schema types (Zod), testing both regular and optional variants.
packages/typebox/package.json (1)
1-46
: LGTM - Package configuration looks goodThe package.json configuration is well-structured for a TypeScript package in a monorepo with appropriate dependencies and build configuration.
packages/typebox/README.md (1)
1-105
: Comprehensive README with clear usage examplesThe README effectively communicates the purpose of the package and provides helpful usage examples.
🧰 Tools
🪛 LanguageTool
[misspelling] ~33-~33: Possible spelling mistakes found.
Context: ...tack Query (React, Vue, Solid, Svelte), Pinia Colada, and more. - 🚀 Server Actions: Ful...(EN_MULTITOKEN_SPELLING_TWO)
[style] ~51-~51: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... your API or implement API contract. - [@orpc/client](https://www.npmjs.com/package/@...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~52-~52: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... API on the client with type-safety. - [@orpc/react](https://www.npmjs.com/package/@o...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~53-~53: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...with React and React Server Actions. - [@orpc/react-query](https://www.npmjs.com/pack...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~54-~54: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...test/docs/framework/react/overview). - [@orpc/vue-query](https://www.npmjs.com/packag...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~55-~55: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...latest/docs/framework/vue/overview). - [@orpc/solid-query](https://www.npmjs.com/pack...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~56-~56: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...test/docs/framework/solid/overview). - [@orpc/svelte-query](https://www.npmjs.com/pac...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~57-~57: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...est/docs/framework/svelte/overview). - [@orpc/vue-colada](https://www.npmjs.com/packa...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[misspelling] ~57-~57: Possible spelling mistakes found.
Context: ...ge/@orpc/vue-colada): Integration with Pinia Colada. - [@or...(EN_MULTITOKEN_SPELLING_TWO)
[style] ~58-~58: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ada](https://pinia-colada.esm.dev/). - [@orpc/openapi](https://www.npmjs.com/package/...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~59-~59: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...I specs and handle OpenAPI requests. - [@orpc/zod](https://www.npmjs.com/package/@orp...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~60-~60: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...tps://zod.dev/) doesn't support yet. - [@orpc/valibot](https://www.npmjs.com/package/...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~61-~61: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...rom Valibot. - [@orpc/arktype](https://www.npmjs.com/package/...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~62-~62: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...from ArkType. - [@orpc/typebox](https://www.npmjs.com/package/...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.17.2)
99-99: Images should have alternate text (alt text)
null(MD045, no-alt-text)
packages/typebox/src/converter.ts (1)
13-15
: LGTM - Condition check is correctly implementedThe
condition
method properly identifies TypeBox schemas by checking for theKind
property.
Uncertain if this is desirable and if I'm missing other edges (I believe validation probably isn't handled correctly). The other alternative is to require using |
// eslint-disable-next-line no-restricted-imports | ||
import type { StandardSchemaV1 } from '@standard-schema/spec' | ||
|
||
export type Schema<TInput, TOutput> = StandardSchemaV1<TInput, TOutput> | ||
|
||
export type AnySchema = Schema<any, any> | ||
export type AnySchema = Schema<any, any> | TSchema |
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.
I'm concerned about this approach because changing the @orpc/contract package directly impacts the core of oRPC. This could create many edge cases we'd need to handle across the entire project. The core should remain as stable and library-agnostic as possible, not designed specifically for one library right now.
My preferred path is to either wait until TypeBox supports standard schema natively or use a temporary wrapper like the one shown here: https://github.com/sinclairzx81/typebox/blob/0d2c1f49d834e3bb2a5a51dd259d57e761785854/example/standard/readme.md.
I believe TypeBox is likely to add standard schema support later.
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.
@unnoq My assumption based on this issue is it will never be a core part of TypeBox, and instead TypeMap
was being proposed as the intended solution for where Standard Schema support is needed.
What I was hoping to achieve DX-wise was to at least be able to loosen the type of .input()
and .output()
so I wouldn't need to wrap every TypeBox type, but I can understand how that trade-off might not seem worthwhile for just 1 non-compliant library.
I do wonder if there could be a more "advanced" approach to creating a custom os
instance that accepts validation plugins, such that non-Standard Schema compliant libraries can still be supported, but I'd have to play around with what that might look like.
Ideally I imagine it would be something as simple as:
import { os } from '@orpc/server'
import { StandardSchemaValidator, TypeBoxValidator } from '@orpc/validators'
export const os = createORPCServer({
validators: [new StandardSchemaValidator(), new TypeBoxValidator()], // type of os.input() and os.output() inferred based on the schema type of provided validator plugins.
});
Under the hood the plugins would implement a similar API to the OpenAPI converters, with a condition
/match
method to check a schema is supported by the plugin, and a convert
/wrap
function that will turn that schema into a Standard Schema compliant object with a validator.
Then hopefully the code change everywhere else would be fairly minimal if the wrapped version of the schema can be created at the top level.
Does that approach sound feasible and/or something you'd be interested in me exploring?
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.
No problem with this approach at runtime, but how would you handle it at the type level? Some magic could work, but a wrapper is still a solid choice.
Good new, I have some idea to make it happen without break current behavior |
@unnoq Awesome! I'm really sorry I should have replied to this PR earlier, I did have a play around and have a half-finished branch introducing the concept of "schema providers". Took me a figure out the type gymnastics to allow some form of HKT to get type inference to work (I took some inspiration from how Fastify type providers work), but it seems like it ultimately can be achieved by just passing in a single provider instance. Only real breaking change would be the new I have a WIP branch I was planning to try finish off in the next few days, otherwise I'm happy to leave it with you. If you did want me to finish it, the current API I was playing with has it as Side note: Another potential benefit I could see is the provider also including a Side side note: The second schema provider I was thinking of adding after TypeBox would be for this library https://typeschema.com/ . It looks a little unmaintained (presumably in part because Standard Schema exists now), but would be the most bang-for-buck to make oRPC compatible with the majority of well known schema validation libraries. |
Thanks @Not-Jayden! I've decided not to include built-in support for TypeBox. Why?
However, if we can find a solution that avoids these conditional checks, we’d be more than happy to support it. Thanks again! |
Adds support for TypeBox types
Summary by CodeRabbit
@orpc/typebox
package, providing a converter for generating OpenAPI specifications from TypeBox schemas..gitignore
for the new package.