sinclairzx81 / typebox

Json Schema Type Builder with Static Type Resolution for TypeScript
Other
4.57k stars 148 forks source link

Non-portable declaration types (in Monorepo) #205

Closed cjol closed 1 year ago

cjol commented 1 year ago

This is probably related to #137 , which never received a public solution. I've got a monorepo set up with two packages: @org/models which exposes some common schemas, and @org/foo-utils which consumes them. They're managed using PNPM, and there's a minimal reproduction available here: https://github.com/cjol/sinclair-repro. Here are the salient code snippets:

@org/models:

import { Type, Static } from '@sinclair/typebox';

export const Foo = Type.Object({
  id: Type.Number()
});
export type Foo = Static<typeof Foo>;

export const FooList = Type.Array(Foo);
export type FooList = Static<typeof FooList>;

@org/foo-utils:

import { Foo, FooList } from '@org/models';

export const getId = (foo: Foo) => foo.id;

export const getIdsArr = (foos: Foo[]) => foos.map(getId);

/**
 * This line causes a type error:
 *     The inferred type of 'getIds' cannot be named without a reference to
 *     'packages/models/node_modules/@sinclair/typebox/typebox'. This is likely not
 *     portable. A type annotation is necessary.ts(2742)
 */
export const getIds = (foos: FooList) => foos.map(getId);

As indicated, the last line causes a type error. I guess it's to do with the packages being linked, but it also does seem limited in scope to StaticArray (the other functions type fine). I'm running pnpm run build in the models directory first, to ensure that declaration files are correctly emitted.

I appreciate that this is likely an environment/configuration issue rather than a bug with typebox, but I am posting here in the hope of finding a solution for future users!

sinclairzx81 commented 1 year ago

@cjol Hi, I think this issue is caused by foo-utils not importing all the necessary type inference infrastructure from TypeBox. It only imports a select few types through the compiled .d.ts.

Below are two options to resolve.

Option (A) Export TypeBox on models project

Below we export all of TypeBox from the models project to ensure that all down stream projects have TypeBox indirectly imported. Note: Remember to re-compile models after adding the line below.

import { Type, Static } from '@sinclair/typebox';

export * from '@sinclair/typebox' // <---------- Added

export const Foo = Type.Object({
  id: Type.Number(),
});
export type Foo = Static<typeof Foo>;

export const FooList = Type.Array(Foo);
export type FooList = Static<typeof FooList>;

Option (B) Import TypeBox types in foo-utils project

Alternatively, you can import all the types in the foo-utils project. This gives TypeScript enough of a hint as to how it should infer for the getIdsArr(...) as it can resolve the dependent type inference infrastructure through this import.

import type * as Types from '@sinclair/typebox' // <---------- Added

import { Foo, FooList } from '@org/models';

export const getId = (foo: Foo) => foo.id;

export const getIdsArr = (foos: Foo[]) => foos.map(getId);

/**
 * This line causes a type error:
 *     The inferred type of 'getIds' cannot be named without a reference to
 *     'packages/models/node_modules/@sinclair/typebox/typebox'. This is likely not
 *     portable. A type annotation is necessary.ts(2742)
 */
export const getIds = (foos: FooList) => foos.map(getId);

Notes

For the mono repositories, I tend towards Option (A) because down stream applications may need to further compose models. Thus Option (A) would allow a down stream applications to do the following.

import { Type, Foo } from '@org/models'

const PartialFoo = Type.Partial(Foo) // <--- Further composition of models

Hope this helps! S

cjol commented 1 year ago

Thank you, re-exporting the types has worked in both my minimal repro and the real-world project. I'm not sure I understand why it works, because the downstream projects should already have had access to typebox directly, but this is not an inconvenient workaround. If there's some space in the documentation, it might be worth adding an reference to it!