refactor(server): migrate to zod schemas (1/x)#26427
Closed
timonrieger wants to merge 15 commits intomainfrom
Closed
refactor(server): migrate to zod schemas (1/x)#26427timonrieger wants to merge 15 commits intomainfrom
timonrieger wants to merge 15 commits intomainfrom
Conversation
|
Label error. Requires exactly 1 of: changelog:.*. Found: 📱mobile, 🖥️web, 🗄️server, preview. A maintainer will add the required label. |
Contributor
|
Deploying preview environment to https://pr-26427.preview.internal.immich.build/ |
Collaborator
Author
|
Doing a full migration PR and some git history cleaning for easier rebases |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
this is the first PR implementing the proposal to migrate away from decorator based dtos to zod schemas. It includes the library setup/integration, test fixes and first dto migration batch.
this PR won't go into detail on the why. Please refer to the discord discussion and #26172.
Note 1: since the migration won't be done in one go, I had to do some limbo due to having to support both approaches now. I have marked them below in my review. They are just temporary and will be removed/cleaned up once the migration is complete.
Note 2: The PR got bigger than I like but it was not feasible without spending more time doing this limbo. The issue is that one dto must either be defined by either the decorator OR zod approach, but never in both (otherwise causing duplicates, weird renaming juggles). I worked myself through the schemas starting from the
UserResponseDtoand migrating all direct and indirect inheritor dtos.Note 3: The primary implementation limitation I set for this is to keep the api generally backwards compatible. See the references for details. Note however that for obvious properties (e.g. counts, uuids) constraints might have been set (e.g. counts are nonnegative). Technically they are breaking yes, but they would have been rejected by the server anyways or are unreasonable, anyways.