Skip to content
This repository has been archived by the owner on Nov 28, 2022. It is now read-only.

Support for allOf and a host of small improvements and bug fixes #436

Merged
merged 54 commits into from
Mar 13, 2020

Conversation

erunion
Copy link
Member

@erunion erunion commented Jan 11, 2020

🌱 Improvements

  • Move react-jsonschema-form off our outdated fork and into a new soft fork at @readme/react-jsonschema-form, which is running the latest version of the upstream project.
  • Backporting Allow schema $refs to not have to start with #/definitions rjsf-team/react-jsonschema-form#954 back into our fork until a future release of RJSF will support it directly.
    • This work is now directly in RJSF and our fork so we no longer need any special handling for component schemas.
  • Adding support for format: blob and format: html on string params to show a textarea.
  • Support allOf in the form.
  • Implementing a hot fix for some oneOf and anyOf UI quirks. See Support for allOf and a host of small improvements and bug fixes #436 (comment) for details.
  • Adding support for allOf, oneOf, and anyOf in response schemas.
    • For oneOf and anyOf cases, 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.
  • Upgrading @readme/oas-tooling to 3.0.0
    • Brings enhancements to handle polymorphism cases in our response schema rendering
    • Fixes a bug where we couldn't handle escaped schema definitions.
    • Updates our JSON Schema generation to function on the newer release of RJSF.
  • Improved error messaging states and the ability for us to capture errors for reporting. feat: improved error messaging functionality #517

🐛 Bug fixes

👨‍🎨 Comparisons

🚧 Known Quirks

  • Swapping between a top-level oneOf state preserves any data in the code sample/request that was set on the other object.
    • ❗️This is a bug in RJSF and out of scope for this PR.
  • Loading a oneOf that contains an allOf causes the form to appear without CSS, but toggling between the dropdown eventually fixes it. Unfortunately toggling another item in the form will break the CSS again.

@erunion erunion added type:enhancement A potential new feature to be added, or an improvement we could make scope:dependency Pull requests that update a dependency file labels Jan 11, 2020
@readmeio readmeio deleted a comment from domharrington Feb 15, 2020
@erunion erunion removed the scope:dependency Pull requests that update a dependency file label Feb 15, 2020
@erunion erunion requested review from dok and kanadgupta March 11, 2020 23:02
@erunion erunion temporarily deployed to api-explorer-feat-mainl-rcvkhk March 11, 2020 23:07 Inactive
Copy link
Member

@domharrington domharrington left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jon, this is fantastic work. Really thorough and detailed description of the work done and the known limitations and issues with the current approach. I had a few questions inline, but on the whole this looks amazing. It's gunna be a game changer for our customers who require this.

I'd love to see a link on a PR app with the polymorphism fully working, with a side by side link to it failing on production!

https://media.giphy.com/media/QMkPpxPDYY0fu/giphy.gif

example/src/Demo.jsx Show resolved Hide resolved
});

describe('schema format handling', () => {
describe('string types', () => {
it('json should render as <textarea>', () => {
it.each([
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woah this is great!

@@ -393,10 +393,12 @@ Doc.propTypes = {
language: PropTypes.string.isRequired,
lazy: PropTypes.bool,
Logs: PropTypes.func,
maskErrorMessages: PropTypes.bool,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this property get sent down from ReadMe?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't done that part yet, but the idea was to expose the isAdmin boolean from hub2 users to a <script data-json="..." /> tag our API Reference template in the same way we populate the rest of the Explorer.

packages/api-explorer/src/Params.jsx Outdated Show resolved Hide resolved
packages/api-explorer/src/Params.jsx Outdated Show resolved Hide resolved
packages/api-explorer/__tests__/PathUrl.test.jsx Outdated Show resolved Hide resolved
@erunion erunion temporarily deployed to api-explorer-feat-mainl-rcvkhk March 12, 2020 19:31 Inactive
@erunion erunion temporarily deployed to api-explorer-feat-mainl-rcvkhk March 12, 2020 22:08 Inactive
@erunion erunion temporarily deployed to api-explorer-feat-mainl-rcvkhk March 12, 2020 22:20 Inactive
@erunion erunion temporarily deployed to api-explorer-feat-mainl-rcvkhk March 12, 2020 22:46 Inactive
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type:bug Something isn't working type:enhancement A potential new feature to be added, or an improvement we could make
Projects
None yet
Development

Successfully merging this pull request may close these issues.

requestBody with oneOf schemas still producing error
3 participants