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

support for class-validator #26

Closed
sachaw opened this issue Mar 12, 2023 · 13 comments
Closed

support for class-validator #26

sachaw opened this issue Mar 12, 2023 · 13 comments
Assignees
Labels
duplicate This issue or pull request already exists enhancement New feature or request

Comments

@sachaw
Copy link

sachaw commented Mar 12, 2023

I have some external classes define the shape of some forms, as they are classes, inferring the shape of a Zod schema is non-trivial, I have previously used https://github.com/typestack/class-validator for validation, but I have run into the situation outlined in #2 are there any other workarounds that could enable this?

@fabian-hiller fabian-hiller self-assigned this Mar 12, 2023
@fabian-hiller
Copy link
Owner

Can you share a code sample with me so that I can understand your problem even better?

@sachaw
Copy link
Author

sachaw commented Mar 12, 2023

Thanks for the reply, here's an outline:

  1. I define a schema that inherits from an external class: https://github.com/meshtastic/web/blob/master/src/validation/config/position.ts inherits from https://js.meshtastic.org/classes/Protobuf.Config_PositionConfig.html
  2. I have a "form generator" that takes a generic(the validation schema from earlier) and a schema to define the physical layout: https://github.com/meshtastic/web/blob/master/src/components/PageComponents/Config/Position.tsx
  3. Finally the generator itself (currently using react-hook-form): https://github.com/meshtastic/web/blob/master/src/components/Form/DynamicForm.tsx https://github.com/meshtastic/web/blob/master/src/components/Form/DynamicFormField.tsx

Hopefully this is clear enough, thanks.

@fabian-hiller
Copy link
Owner

Is your problem that the derived type is an interface and Modular Forms throws an error when you pass it as generic or does your problem relate to validating the form inputs?

@sachaw
Copy link
Author

sachaw commented Mar 12, 2023

The former. I am unable to use your previous guidance of using a type instead of an interface

@fabian-hiller
Copy link
Owner

Unfortunately, I don't know any solution for this at the moment. To make the form values type-safe even with nested objects and arrays, I use a recursive type and this approach leads to problems when using interfaces. I will ask for advice in the TypeScript Discord. However, it is uncertain if there is a solution for this as described in #2. I will report here as soon as I have an answer.

@fabian-hiller fabian-hiller added bug Something isn't working enhancement New feature or request duplicate This issue or pull request already exists labels Mar 13, 2023
@fabian-hiller
Copy link
Owner

I received an answer in the TypeScript Discord and am now checking if the given solution works for Modular Forms.

@fabian-hiller
Copy link
Owner

I have been busy with other things the last few days. I hope to give you feedback on this over the weekend.

@sachaw
Copy link
Author

sachaw commented Mar 16, 2023

No rush, thanks man

@fabian-hiller
Copy link
Owner

Sorry. I haven't gotten around to it yet. I'll get back to you as soon as I find time for it.

@fabian-hiller fabian-hiller removed the bug Something isn't working label Apr 3, 2023
@sachaw
Copy link
Author

sachaw commented May 7, 2023

Found a workaround:
The yup validation library has ObjectSchema that allows deriving a schema from an external class definition.
So maybe this issue can be converted to requesting support for yup

@fabian-hiller
Copy link
Owner

Thanks for the info! Do you have a code example? Do you also work with Yup and need adapters for validation with Modular Forms?

@fabian-hiller
Copy link
Owner

All solutions I know so far to be able to support interfaces reduce type safety. I have again experimented a bit, but have not come to a satisfactory solution.

@fabian-hiller
Copy link
Owner

I'm temporarily closing the issue until there are signs that changes to our code can fix the problem.

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

No branches or pull requests

2 participants