Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change omit() parameter type to readonly TKeys[] #380

Closed
pschiffmann opened this issue Jan 21, 2024 · 4 comments
Closed

Change omit() parameter type to readonly TKeys[] #380

pschiffmann opened this issue Jan 21, 2024 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@pschiffmann
Copy link

Hey! Great package, I really enjoy using it, and I like the elegant yet expressive API! :)

I'd like to request a minor API change regarding the omit() method. Please change the signature to omit(schema: TSchema, keys: Readonly<TKeys>).

Example use case

I'm using valibot for tRPC input validation. Here is a simplified example of my backend setup:

const InstantSchema = transform(string(), (s) => Temporal.Instant.from(s));
const entityMixin = {
  id: string([uuid()]),
  createdAt: InstantSchema,
  updatedAt: InstantSchema,
};

const FooSchema = object({
  ...entityMixin,
  x: string(),
  y: number(),
});

const omitProps = {
  create: ["id", "createdAt", "updatedAt"],
  update: ["createdAt", "updatedAt"],
} as const;

const t = initTRPC.create();
const fooRouter = t.router({
  create: t.procedure
    .input(wrap(omit(FooSchema, omitProps.create)))
    .output(wrap(FooSchema))
    .mutation(() => {
      // ...
    }),

  update: t.procedure
    .input(wrap(omit(FooSchema, omitProps.update)))
    .output(wrap(FooSchema))
    .mutation(() => {
      // ...
    }),
});

The code above generates TypeScript errors for the omit() calls: The type 'readonly ["id", "createdAt", "updatedAt"]' is 'readonly' and cannot be assigned to the mutable type 'ObjectKeys<...>'..

Why even pass a readonly array to omit()?

I have a couple of different entities in my data model, all share the same common properties defined in entityMixin. I also have CRUD endpoints for all of these entities that work pretty much the same: The create mutations receive a property bag, generate an ID for the new entity, and return the full object.

Because all create and update mutations omit the same properties from their input type, I declared that key array as a reusable variable in omitProps.create. By declaring that variable as const, it's type becomes readonly ["id", "createdAt", "updatedAt"]. I can't remove the as const, because then typeof omitProps.create would become string[], and the specific property name literals would get lost.


I'm only using omit() at the moment, so for me it would be sufficient to fix this one method for now. If you want me to, I can go over the remaining API and check for other places that should also be changed (for API consistency).

I can open a PR if you want, just wanted to check first if you're even interested in this change.

@fabian-hiller
Copy link
Owner

Thanks for bringing this up. I will update the types of omit and pick and add our MaybeReadonly util type to support both cases. Feel free to create more issues if you encounter problems or have ideas on how we can further improve the library.

@fabian-hiller fabian-hiller self-assigned this Jan 21, 2024
@fabian-hiller fabian-hiller added the enhancement New feature or request label Jan 21, 2024
@pschiffmann
Copy link
Author

Thanks, this is exactly what I needed!

@fabian-hiller
Copy link
Owner

I will release a new version in the next days. If you don't want to wait, you can build the library with the changes yourself. Contact me if you want to do that and need help.

@fabian-hiller
Copy link
Owner

v0.27.0 is available

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants