fabian-hiller / valibot

The modular and type safe schema library for validating structural data 🤖
https://valibot.dev
MIT License
6k stars 186 forks source link

feat: support metadata in pipe #655

Closed xcfox closed 2 months ago

xcfox commented 3 months ago

Implements https://github.com/fabian-hiller/valibot/issues/373, support metadata in pipe and pipeAsync.

In this implementation, metadata is used as previously discussed

// With the new `pipe` function
const UserSchema = pipe(
  object({
    id: pipe(string(), uuid(), primaryKey()),
    name: pipe(string(), maxLength(32), unique()),
    bio: pipe(string(), description('Text ...')),
  }),
  table('users')
);

Metadata can add extra properties on the schema:

const schema = pipe(
  object({
      key: string(),
  }),
  withDescription('some Description')
)
console.log(schema.description) // 'some Description'

Currently, I've added two metadata:

fabian-hiller commented 3 months ago

Sorry for not getting back to you in #373, but I am still working on other areas of the library as a result of our rewrite, and the metadata feature is not something I want to rush. It is important to me to make the right decisions. As soon as I find the time, I will answer you in the issue.

fabian-hiller commented 2 months ago

Sorry for the late reply. I took a week off. I have started to review our conversations as well as this PR. If we decide to implement the metadata feature as part of our pipe method, is it really necessary to extract the extraProperties? I ask because this could also be the job of the library that processes the schema anyway. This way we could reduce our bundle size as well as the initialization performance if this feature is not used.

xcfox commented 2 months ago

In my opinion, Extracting the extraProperties would be a small improvement to the development experience.

An example of tRPC is:

import * as v from 'valibot'
const appRouter = router({
  userById: publicProcedure
  // Extract the `extraProperties` from the `v.string()` schema
  .input(v.pipe(v.string(), v.uuid(), v.asParser()))
  .query(async (opts) => {
    const { input } = opts
    // Retrieve the user with the given ID
    const user = await db.user.findById(input)
    return user
  }),
})
import * as v from 'valibot'
const appRouter = router({
  userById: publicProcedure
  // nested directly
  .input(v.parser((v.pipe(v.string(), v.uuid()))))
  .query(async (opts) => {
    const { input } = opts
    // Retrieve the user with the given ID
    const user = await db.user.findById(input)
    return user
  }),
})

When adding more than two derivations to a schema, the benefits become more apparent:

import * as v from 'valibot'

// with the `extraProperties`
const Giraffe = v.pipe(
  v.object({
    name: v.string(),
    age: v.number(),
  }),
  v.asParser(),
  asObjectType(),
  asEntity()
)

// without the `extraProperties`
const Giraffe = v.object({
  name: v.string(),
  age: v.number(),
})
const GiraffeParser = v.asParser(Giraffe)
const GiraffeType = asObjectType(Giraffe)
const GiraffeEntity = asEntity(Giraffe)

Extract the extraProperties make the code more readable and maintainable. And I don't think doing this increases the bundle size, it's just type.

fabian-hiller commented 2 months ago

And I don't think doing this increases the bundle size, it's just type.

I think it adds this to the bundle. This can result in a 50% increase from 200 to 300 bytes.

  const extraProperties = pipe.reduce(
    (props, it) => {
      if ('extraProperties' in it) Object.assign(props, it.extraProperties);
      return props;
    },
    {} as ExtraProperties<[TSchema, ...TItems]>
  );

As I mentioned, I am not sure we should do it this way. I am also not sure if we should stick to the extraProperties property for each metadata action, as it does not fit well with the rest of the code (but I think I know why you did it that way). I would prefer to add such properties directly to the action object.