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

[swagger-zod] readOnly properties are omitted using .and, creating a intersection of never and original type #943

Open
aburgel opened this issue Apr 18, 2024 · 7 comments
Labels
bug Something isn't working @kubb/plugin-zod

Comments

@aburgel
Copy link
Contributor

aburgel commented Apr 18, 2024

What version of kubb is running?

kubb/2.13.0 darwin-arm64 node-v20.11.1

What platform is your computer?

No response

What version of external packages are you using(@tanstack-query, MSW, React, Vue, ...)

zod: 3.22.4

What steps can reproduce the bug?

I have some readOnly properties, and the resulting zod schema attempts to omit them by using .and(z.object({ id: z.never() }). This results in an intersection which means both schemas have to pass. For a never and some other type, that isn't possible so the validation always fails. See https://zod.dev/?id=and for more on .and

zod has an .omit function that might be a better fit here: https://zod.dev/?id=pickomit

How often does this bug happen?

Every time

What is the expected behavior?

No response

Swagger/OpenAPI file?

No response

Additional information

No response

@aburgel aburgel added the bug Something isn't working label Apr 18, 2024
@stijnvanhulle
Copy link
Collaborator

z.omit would not work for readonly properties. Those should still be visible in the schema but not be settable. Omit will remove the field, which is not something we want.

Not sure what we can do here with that readonly does not exist in Zod for one specific item(https://zod.dev/?id=readonly). That is only possible for the full object.

@aburgel
Copy link
Contributor Author

aburgel commented Apr 20, 2024

Another option is to use .extend instead of .and since that will overwrite the existing type instead of producing an intersection.

So you could just replace what is generated now with .extend(z.object({ id: z.never() }) and that should fail if id is present.

@stijnvanhulle
Copy link
Collaborator

Extend only works on another object so we need to remove the zod.lazy and regenerate the Zod model when a read-only field is used. This makes it a little bit harder to fix it so not really sure what the best solution will be for this(without a big impact).

@LiamMartens
Copy link

@stijnvanhulle this actually makes sense since zod implements .readonly() as an Object.freeze() but this wouldn't be possible with a partial object anyways - at least not from a runtime perspective.

This will probably just be a type-only implementation which could be achieved with .refine();

Maybe something like below which will result in a partial readonly typed object:

type SomeReadonly<Obj, Keys extends keyof Obj> = (
  Pick<Obj, Exclude<keyof Obj, Keys>>
  & Readonly<Pick<Obj, Keys>>
)

z.object().refine((input): input is SomeReadonly<typeof input, ReadonlyKeys> => {
  return true;
})

I can have a look at creating a PR if you want, sometime this week

@aburgel
Copy link
Contributor Author

aburgel commented Apr 23, 2024

That might be a good option in some cases. However it’s a little unclear to me what the goal is.

My goal is to have a validator that excludes readOnly properties so I can validate a form. I don’t really care if the form object actually contains those properties because my backend will ignore them. I’m trying to avoid defining a whole other set of objects in my schema that exclude those readOnly fields.

I don't need a type to enforce the read only status on a given object. Perhaps that’s useful for others cases tho.

@peter-job-avb
Copy link

Based on specification here:

Relevant only for Schema "properties" definitions. Declares the property as "read only". This means that it MAY be sent as part of a response but SHOULD NOT be sent as part of the request. If the property is marked as readOnly being true and is in the required list, the required will take effect on the response only. A property MUST NOT be marked as both readOnly and writeOnly being true. Default value is false.

I'd expect the property to not be visible in the request schema, only the response schema. The zod omit method seems appropriate here, rather than readonly.

@ollibolli
Copy link

ollibolli commented Oct 1, 2024

I have the same problem. Readonly turn to .and(z.object({ id: z.never() }) that ends up with a condition that can't be met.

If you look at the type def generated by the swagget-ts plugin it interpret it as
export type CreateResourceMutationRequest = Omit<NonNullable<Resource>, "id">;

im my case the omit should be more appropriate. Where i do not need to expose this option in the request body.

Made a little test for alternative implementation

import { z } from 'zod';

const idSchema = z.string();
const resourceSchema = z.object({
  id: z.lazy(() => idSchema).optional(),
  name: z.string(),
  relation: z.string().optional(),
});

describe('Readonly parsing with omit', () => {
  const createresourceMutationRequestSchema = z.lazy(() =>
    resourceSchema.omit({ id: true }),
  );

  test('Null id', () => {
    const res = createresourceMutationRequestSchema.parse({
      id: null,
      name: 'TestMock',
      relation: 'Significant other',
    });
    expect(res).toEqual({
      name: 'TestMock',
      relation: 'Significant other',
    });
  });
  test('No id', () => {
    const res = createresourceMutationRequestSchema.parse({
      name: 'TestMock',
      relation: 'Significant other',
    });
    expect(res).toEqual({
      name: 'TestMock',
      relation: 'Significant other',
    });
  });
  test('Faulty id', () => {
    const res = createresourceMutationRequestSchema.parse({
      id: 'faulty',
      name: 'TestMock',
      relation: 'Significant other',
    });
    expect(res).toEqual({
      name: 'TestMock',
      relation: 'Significant other',
    });
  });
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working @kubb/plugin-zod
Projects
None yet
Development

No branches or pull requests

5 participants