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

Fix language check #794

Closed
wants to merge 1 commit into from
Closed

Conversation

browniefed
Copy link

Description

This seemed to be breaking on docusaurus v3.2.0

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

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.

@sserrata
Copy link
Member

sserrata commented Apr 4, 2024

Hi @browniefed, I just tried upgrading to Docusaurus 3.2.0 using a test site and was not able to reproduce the error you described - it worked as expected the first time. Can you provide steps to reproduce the error along with additional context?

@browniefed
Copy link
Author

Hi @browniefed, I just tried upgrading to Docusaurus 3.2.0 using a test site and was not able to reproduce the error you described - it worked as expected the first time. Can you provide steps to reproduce the error along with additional context?

I'm not certain exactly what the setup is, but I would assume some of our code samples aren't getting annotated and thus a language isn't being chosen?

If you look here https://github.com/PaloAltoNetworks/docusaurus-openapi-docs/pull/794/files#diff-d540d718a2f059832b0aa4da83589d1c6ec8110b6758874abed3da6ab8747943R333

Literally after my language && check there is a check for if the language is null, so it clearly has a possibility of being null.

Same with other useEffects in the file.

So I am happy to take a moment to spin up a repro, it would be nice to have this just matching all of the other code in the file.

The other option is just move the null check on language above the thing I did and I think that would then solve the problem.

@sserrata
Copy link
Member

sserrata commented Apr 4, 2024

Adding a safety check is low risk but I would prefer to reproduce the error so I can validate the fix. Can you provide a copy of the OpenAPI spec or a snippet?

@browniefed
Copy link
Author

@sserrata here is the swagger.json file we use to generate https://gist.github.com/browniefed/4dd6cb7f6fbfe295ad6be59506f3cc5d

If you can't repro then I can attempt to set something up later.

When I was running v3.1.1 of docusaurus I got a useDoc hook issue for non-openapi created pages seemingly.
Then on v3.2.0 everything was working, minus this language.sampels thing.

@sserrata
Copy link
Member

Hi @browniefed, I've been unable to reproduce, even with the provided spec on Docusaurus 3.3.2. I'm moving to close this PR but if you're able to provide updated repro instructions feel free to reopen as an issue. Thanks!

@sserrata sserrata closed this May 14, 2024
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