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

introducing OmitReadonly type to resolve required + readonly/writeonly properties #1145

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tiholic
Copy link

@tiholic tiholic commented Jul 16, 2022

closes #432

Pretext:

Say there are 2 properties to a model id and name. Creating a model instance (POST) requires only name to be posted and id would be a readonly property. Generated type would be

type Model {
  readonly id: number;
  name: string;
}

To create an instance of Model for POST, we should be able to pass {name: "john"}
Generated services look like: createMode(model: Model) {...}, which do not accept an object with just name property.

Solution:

This PR aims to solve this issue by adding a type modifier, so effectively generated service code looks like: createMode(model: OmitReadOnly<Model>) {...} which strips off any readonly properties and allows passing an object with just the name property.

More about OmitReadonly:

  • it omits readonly properties from an object
  • it infers the original type in case of passed type is not an object
  • it is applied to parameters for "POST", "PUT" or "PATCH" operations and only if it is a "reference" type

Implementation based on #432 (comment) by @sweethuman

@tiholic tiholic changed the title introducing OmitReadonly type introducing OmitReadonly type to solve required + readonly/writeonly properties Jul 16, 2022
@tiholic tiholic changed the title introducing OmitReadonly type to solve required + readonly/writeonly properties introducing OmitReadonly type to resolve required + readonly/writeonly properties Jul 16, 2022
- it omits readonly properties from an object
- it infers the original type in case of passed type is not an `object`
- it is applied to parameters for "POST", "PUT" or "PATCH" operations
and only if it is a "reference" type
@aaronschif
Copy link

aaronschif commented Aug 23, 2022

Please change the OmitReadonly imports to type imports. Vite will complain otherwise. Thank you.

https://github.com/ferdikoomen/openapi-typescript-codegen/pull/1145/files#diff-33d2b8dc9fd33ef2b353540e2ff031e54952e1013569382346cab750d11f9da2R33

@jeremyzahner
Copy link

We desperately need this. =)

@tiholic Any chance we can get the type import fixed?
@ferdikoomen Can we get this merged after?

I'm happy to help. Let me know if I can support in any way.

@tiholic
Copy link
Author

tiholic commented Oct 27, 2022

@aaronschif @jeremyzahner import converted to a type import

@ferdikoomen I believe this can be prioritised now?

@jeremyzahner
Copy link

@tiholic @aaronschif @demo-exe for the time being, you can use https://www.npmjs.com/package/@jshmrtn/openapi-typescript-codegen until @ferdikoomen can have a look.

@giopetris
Copy link

Any chance to get this merged?

@dacox
Copy link

dacox commented Oct 5, 2023

@tiholic this is interesting. It appears that openapi-typescript-codegen is marking readonly fields as optional, which I think solves the problem you outlined above (not being able to just do createMode({.name }: Model) {...}

We are facing almost the opposite problem - because these readonly fields are optional, we are getting the response model type containing things like optional ids, which we definitely don't want

@cbowal
Copy link

cbowal commented Feb 18, 2024

Bumping @giopetris's request – would it be possible to get this merged?

@tajnymag
Copy link
Contributor

tajnymag commented Mar 2, 2024

@mrlubos Maybe this could be merged into your fork?

@mrlubos
Copy link
Collaborator

mrlubos commented Mar 2, 2024

@tajnymag do you mind opening an issue in our repository? I'd like to solve this with write-only too, this pull request seems to work only for read-only

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

Successfully merging this pull request may close these issues.

required and readyOnly/writeOnly properties
9 participants