feat(core): Only consider non-readonly properties in request bodies #813
Conversation
|
Someone is attempting to deploy a commit to a Personal Account owned by @anymaniax on Vercel. @anymaniax first needs to authorize it. |
maikdijkstra
changed the title
|
@anymaniax I updated the PR, so now we keep track of whether the type contains any readonly properties, if it contains a readonly property anywhere in the object tree then the |
|
@maikdijkstra thanks a lot. Seems great. I found it weird to inject it in the index.ts. What do you think about inject it in each body file that need it? I was thinking would this need to be deactivated for some case? I don’t think so but what is your thoughts about this? I thought also about another solution. Be able to do the same thing as with a mutator but without the need to define all the mutator. |
|
@anymaniax My reason to inject it into the index.ts is that it will not show up with any other schemas in a separate file but is importable from the schemas folder and therefore only generate this type once. Does the body file mean the file with the endpoints? If so, then yes that is also an option, you will get quite some duplication if you generate an endpoint per file and all of them need it. I think this case should not be deactivatable, since then the specification is simply wrong according to the OpenApi specification. I don't really follow your proposed solution using the mutator. Could you explain how that would work? |
|
@maikdijkstra ok I think it’s good enough for the moment. Can you just add an example with it and a test in the tests folder? For the second solution it wouldn’t solve the main problem directly so it doesn’t make sense forget about it 😅. Again thanks for the help |
Readonly properties only have effect on responses. So for request bodies the properties that are readonly should be removed from the type.
|
@anymaniax Good to hear! I added a test case to the default config. I also updated the react-query/basic sample to have a readonly property in the request body |
|
Super @maikdijkstra I plan to checkout and test the branch tomorrow and merge it after that. I will create a new release with it during the week. |
|
I was testing this and found that export type NonReadonly<T> = {
[P in keyof Writable<T>]: T[P] extends number | string | boolean | null | undefined
? T[P]
: NonReadonly<NonNullable<T[P]>>;
};Here is a small reproduction: type Body = { id?: string, name: { first?: string, last: string | null }};
// Type 'string' is not assignable to type 'NonReadonly<string>'.ts(2322)
const obj: NonReadonly<Body> = { id: 'abd', name: { last: null} }Above, the inline object is valid but can't be assigned to |
|
@maikdijkstra for me all is good when you have time can you check the comment of @localrobot |
@anymaniax, @localrobot - I think this was an issue in a previous version. I found this issue myself too so I changed the implementation to instead check whether it is a primitive reverse the statement to whether it extends object. This fixes this issue, so it should work in the latest version of this PR. type IfEquals<X, Y, A = X, B = never> = (<T>() => T extends X ? 1 : 2) extends <
T,
>() => T extends Y ? 1 : 2
? A
: B;
type WritableKeys<T> = {
[P in keyof T]-?: IfEquals<
{ [Q in P]: T[P] },
{ -readonly [Q in P]: T[P] },
P
>;
}[keyof T];
type UnionToIntersection<U> = (U extends any ? (k: U) => void : never) extends (
k: infer I,
) => void
? I
: never;
type DistributeReadOnlyOverUnions<T> = T extends any ? NonReadonly<T> : never;
type Writable<T> = Pick<T, WritableKeys<T>>;
export type NonReadonly<T> = [T] extends [UnionToIntersection<T>]
? {
[P in keyof Writable<T>]: T[P] extends object
? NonReadonly<NonNullable<T[P]>>
: T[P];
}
: DistributeReadOnlyOverUnions<T>;
type Body = { id?: string, name: { first?: string, last: string | null }};
// Type 'string' is not assignable to type 'NonReadonly<string>'.ts(2322)
const obj: NonReadonly<Body> = { id: 'abd', name: { last: null} }This version is valid typescript |
|
Thanks a lot @maikdijkstra |
|
Will we get the same functionality for |
Status
READY
Description
Whenever a readonly property is defined in a request body then these may be sent as part of a response but should not be sent as part of a request according to the openapi 3.0 specification. To solve this using types, we can only allow the properties that are non-readonly since these are the actual request body.
Should fix: #616
Steps to Test or Reproduce
Outline the steps to test or reproduce the PR here.