-
-
Notifications
You must be signed in to change notification settings - Fork 240
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
fix: schema-path handles falsy values and JSON path expression as field #917
Conversation
expect(runSchemaPath(target, fieldToCheck, path)).toEqual([ | ||
{ | ||
path: ['example'], | ||
message: '{{property|gravis|append-property|optional-typeof}}type should be string', |
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 is this syntax? Does type need a space before it?
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's replaced at runtime, when Spectral processes messages returned by functions.
|
acts kind of like the unix pipe.
So in this case, this is what happens:
property
prints the property namegravis
is a function that decorates a property with gravises (\
property``)append-property
adds\
${property} property`` if there is a property or return empty string if there is no property- optional-typeof prints a type of linted value if
append-property
returned an empty string or the result of append-property.
I know this sounds weird, but so perhaps a note in documentation would be nice to have?
What user receives is something like
message: '`example` property type should be number',
or
message: `number should be equal to one of the allowed values: 1, 3, 5, 10, 12`,
That does make sense?
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.
If this is something users should know about then please let's document it.
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.
This also confused me a lot when working on my recent PRs....
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.
If this is something users should know about then please let's document it.
Will do!
This also confused me a lot when working on my recent PRs....
My apologies! Going to document that syntax this week.
]); | ||
}); | ||
|
||
it('given non-existing property, returns the whole document', () => { |
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.
@P0lip Either the test title is misleading or there's something I don't understand. Where is the "whole document" in the provided output?
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.
ah, I should have stated input
Fixes #916
Checklist
Does this PR introduce a breaking change?