-
Notifications
You must be signed in to change notification settings - Fork 159
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
(feat): refactor how pagination properties are checked in fern check
#4250
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.
Refactor looks great overall, nicely done!
We might be able to do something similar in the IR generator since we have to support a similar analogue abstraction there (i.e. the PropertyResolver).
That refactor isn't as necessary as this one, so we could also defer that until we actually unify fern check
and fern ir
since it otherwise might be a little wasteful.
propertyValidator: { | ||
propertyID: "has-next-page", | ||
validate: isValidHasNextPageType | ||
path: getPathFromSelector(offsetPagination["has-next-page"]), |
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.
It looks like this would support any selector prefix - it doesn't actually validate that the user specified $response
.
@@ -151,23 +152,22 @@ function validateHasNextPageProperty({ | |||
if (offsetPagination["has-next-page"] == null) { |
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.
nit: A variable would be nice for this since we use offsetPagination["has-next-page"]
in a few places here.
We previously referred to them as propertyComponents
, but we could probably just do path
so it's consistent with the new validatePropertyInType
signature.
Introduces a better primitive for
property
paths are checked.$response.hasNextPage points to a string but should point to a boolean instead
validatePropertyInType
invokers control the error message