Conversation
domharrington
left a comment
domharrington
left a comment
There was a problem hiding this comment.
👐 few questions in-line, but it looks great on the whole.
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "extends": ["@commitlint/config-conventional"] | |||
There was a problem hiding this comment.
Helps conform commits to a common standard https://commitlint.js.org/. I've been manually adopting this in our OSS projects because it helps to keep changelogs and release notes consistent, this just formalizes that here.
|
|
||
| test('should return a definition for a given ref that is escaped', () => { | ||
| expect( | ||
| findSchemaDefinition('#/components/schemas/Pet~1Error', { |
There was a problem hiding this comment.
Is this pattern of escaping actually what happens in openapi? Is this supported? Pet~1Error
There was a problem hiding this comment.
Yeah, to have a a schema, Pet/Error, with a slash in it you need to escape the / as ~1, otherwise trying to access #/components/schemas/Pet/Error would try to load:
components: { schemas: { Pet: { Error: { ... } } } }
jsonpointer handles this all for us. 🚀
| }); | ||
|
|
||
| describe('polymorphism cases', () => { | ||
| describe('allOf', () => { |
There was a problem hiding this comment.
So flattenSchema is for use in the response sidebar? 👌
There was a problem hiding this comment.
Yep, that's the only place it's used.
| }); | ||
|
|
||
| describe('anyOf', () => { | ||
| it('should flatten only the first schema listed', () => { |
There was a problem hiding this comment.
This is good for now! We can work on this in future to allow selection in the UI.
| label: types[type], | ||
| schema: oas.components | ||
| ? { definitions: { components: cleanupSchemaDefaults(oas.components) }, ...cleanupSchemaDefaults(schema.schema) } | ||
| ? { components: cleanupSchemaDefaults(oas.components), ...cleanupSchemaDefaults(schema.schema) } |
There was a problem hiding this comment.
Is this how you're supposed to do it in OAS? Did I misread the spec?
There was a problem hiding this comment.
Nope! This change was needed because the version of your #/definitions handling in our fork got merged last week. rjsf-team/react-jsonschema-form#1506
RJSF can now properly access #/components/schemas/... without trying to think that they should be located under #/definitions/components/schemas/....
Bumps [@commitlint/config-conventional](https://github.com/conventional-changelog/commitlint/tree/HEAD/@commitlint/config-conventional) from 17.0.2 to 17.0.3. - [Release notes](https://github.com/conventional-changelog/commitlint/releases) - [Changelog](https://github.com/conventional-changelog/commitlint/blob/master/@commitlint/config-conventional/CHANGELOG.md) - [Commits](https://github.com/conventional-changelog/commitlint/commits/v17.0.3/@commitlint/config-conventional) --- updated-dependencies: - dependency-name: "@commitlint/config-conventional" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
3 participants
find-schema-definition
$refpointers that are escaped with~1by just using the great jsonpointer library.find-schema-definitionif we find a nested$ref. We handle that case in the code that handlesfind-schema-definitionin the Explorer, and though they're probably ripe for a rewrite it's fine for now.flatten-schema
oneOf,allOf, andanyOfpolymorphism cases.oneOfandanyOfcases, since we can't really flatten down every option defined in those arrays into a single schema that would validate against the document, I'm choosing to just select the first defined object in the set and flatten that out.parameters-to-json-schema
definitionsobject as a recent push to RJSF made this no longer a requirement.