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

Removing strict objects from Zodios clients generation #966

Closed
wants to merge 2 commits into from

Conversation

ecamellini
Copy link
Collaborator

@ecamellini ecamellini commented Sep 12, 2024

Closes IMN-823

This PR removes strictObjects to avoid .strict() on the generated Zod types used by Zodios.

Why?

As we discussed, there are cases where we could be adding fields to an object that are not part of the model, without TS complaining - see this TS playground example

Thus, we must remove .strict() from Zod validators because it could cause a runtime error. For example:

  1. We are in the BFF
  2. We add some extra properties to an object without TS complaining at compile time
  3. We pass that object as the body of an API call to a process (e.g., the catalog process)
  4. The catalog process validates the body in input and gives a runtime error due to the .strict() validator and the extra properties, the API call fails

Without .strict(), the runtime validators strip the extra properties without erroring, which prevents these potential bugs.

@sandrotaje sandrotaje marked this pull request as ready for review September 16, 2024 14:47
@sandrotaje sandrotaje marked this pull request as draft September 16, 2024 14:51
@ecamellini ecamellini marked this pull request as ready for review September 17, 2024 09:01
@ecamellini ecamellini changed the title Removing strict objects Removing strict objects from Zodios clients generation Sep 17, 2024
Copy link
Collaborator

@MalpenZibo MalpenZibo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

@ecamellini
Copy link
Collaborator Author

Waiting for the discussion we are having off GitHub with @galales / @beetlecrunch, we may decide to keep strict and close this PR.

@ecamellini
Copy link
Collaborator Author

Closing, we decided to keep the .strict() validator

@ecamellini ecamellini closed this Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants