Skip to content
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

Format JSX with newlines and indentation #648

Closed

Conversation

sangaline
Copy link

@sangaline sangaline commented Oct 22, 2023

Description

This modifies how generated JSX is formatted such that it includes newlines and indentation. Previously, much of the content would be on a single line. Here's an example of the new output:

<MimeTabs className={'openapi-tabs__mime'}>
  <TabItem label={'application/json'} value={'application/json-schema'}>
    <details
      style={{}}
      className={'openapi-markdown__details mime'}
      data-collapsed={false}
      open={true}
    >
      <summary style={{}} className={'openapi-markdown__details-summary-mime'}>
        <h3 className={'openapi-markdown__details-summary-header-body'}>Body</h3>
        <strong className={'openapi-schema__required'}>required</strong>
      </summary>
      <div style={{ textAlign: 'left', marginLeft: '1rem' }}></div>
      <ul style={{ marginLeft: '1rem' }}>
        <SchemaItem
          collapsible={false}
          name={'username'}
          required={true}
          schemaName={'Username'}
          qualifierMessage={undefined}
          schema={{ title: 'Username', type: 'string' }}
        ></SchemaItem>
        <SchemaItem
          collapsible={false}
          name={'password'}
          required={true}
          schemaName={'Password'}
          qualifierMessage={undefined}
          schema={{ title: 'Password', type: 'string' }}
        ></SchemaItem>
      </ul>
    </details>
  </TabItem>
</MimeTabs>

Motivation and Context

There are two motivations:

  1. It's easier to read the generated MDX, and diffs are also much cleaner and easier to read when regenerating docs.
  2. The new formatting is compatible with @mdx-js/react v1 and v2. The previous formatting was only compatible with v1, and is one of the blockers to docusaurus v3 compatibility. See MDX compilation fails with Docusaurus v3 #591 for an open issue.

How Has This Been Tested?

I've tested this manually with my own API documentation.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes if appropriate.
  • All new and existing tests passed.

@welcome-to-palo-alto-networks

🎉 Thanks for opening this pull request! We really appreciate contributors like you! 🙌

@sangaline
Copy link
Author

I've discovered that this can break markdown formatting of response descriptions with mdx v1 because they're no longer on their own lines without indentation. Is there any plan to have a branch for docusaurus v3 compatibility? This might be a better fit for that. If not, I can try to preserve the mdx v1 behavior while still adding support for v2, the generated code will just be a bit uglier.

@sserrata
Copy link
Member

Hi @sangaline, thanks for PR and suggestions.

If you haven't seen it already, I would recommend reading through #514 for additional background/work on this issue, which resulted in https://docusaurus-openapi.tryingpan.dev/#markdowngenerators.

We haven't yet identified/explored the breaking changes introduced between Docusaurus 2.4.1 - v3.0.0. If there's some path to maintaining backward compatibility, we may explore that but, based on past experience, that may not be possible without a major refactor.

@github-actions
Copy link

Visit the preview URL for this PR (updated for commit fc40851):

https://docusaurus-openapi-36b86--pr648-xsrmisq1.web.app

(expires Wed, 22 Nov 2023 18:53:21 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@sserrata
Copy link
Member

Hi @sangaline, any luck with solving for the broken descriptions?

@sserrata
Copy link
Member

See #660 for implementation of your changes

@sserrata sserrata closed this Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants