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
refactor(npm): Use schema for PackageSource
parsing
#22314
refactor(npm): Use schema for PackageSource
parsing
#22314
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the advantage of this approach? It's longer code, and less readable to a non-zod person. So is there a "bigger picture" advantage?
It helps to decouple data validation (declarative part) and data transformation. They're separated, but often live together since usually we want them to be performed one right after another. Refactored code inside ...
if (homepage) {
result.homepage = homepage;
}
if (sourceUrl) {
result.sourceUrl = sourceUrl;
}
... One reason why type schema and its data transformation are better to live together: the only place when user can use the outer-world type (e.g., from One more reason for this approach is the granularity and composability: we don't want to just validate the whole nested data structure against the strict schema. We want to do the best for accepting the incomplete data as the valid structure work we can work with: default values, catch clauses and logging the specific information. We don't want to reject all the data just because some not important field is invalid. Once done with particular fields, we then combine them together into the single structure for further usage. The downside is the syntax that is hard to parse without prior knowledge of the library. But code I'm refactoring very often also is not easy to read. Seems like we're choosing between two types of hard to read code, but with new approach providing stronger type safety guarantees, while for another one we often assume everything will be okay and types we're describing are matching the data we receive from the outside world. |
BTW I think it would be way easier to read if the |
🎉 This PR is included in version 35.96.2 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Changes
Context
Documentation (please check one with an [x])
How I've tested my work (please select one)
I have verified these changes via: