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

ValidationError raised for empty or missing navbar.title #3117

Closed
sserrata opened this issue Jul 24, 2020 · 5 comments · Fixed by #3120
Closed

ValidationError raised for empty or missing navbar.title #3117

sserrata opened this issue Jul 24, 2020 · 5 comments · Fixed by #3120
Labels
bug An error in the Docusaurus core causing instability or issues with its execution

Comments

@sserrata
Copy link
Contributor

🐛 Bug Report

The following errors were observed after upgrading to alpha 59:

  • ValidationError: "navbar.title" is not allowed to be empty
  • ValidationError: "navbar.title" is required

Currently, none of our D2 sites use navbar.title as we rely heavily on our logos to identify our sites. I suspect the same might be true for other D2 sites listed under Showcase.

Have you read the Contributing Guidelines on issues?

Yes.

To Reproduce

(Write your steps here:)

  1. Upgrade to alpha 59
  2. Attempt to start dev server with an empty navbar.title value or missing navbar.title key.

Expected behavior

I expect navbar.title to be optional and/or accept an empty string value.

(Write what you thought would happen.)

Actual Behavior

The following errors were observed:

  • ValidationError: "navbar.title" is not allowed to be empty
  • ValidationError: "navbar.title" is required

(Write what happened. Add screenshots, if applicable.)

Your Environment

  • Docusaurus version used: alpha.59
  • Environment name and version (e.g. Chrome 78.0.3904.108, Node.js 10.17.0): Chrome 84.0.4147.89
  • Operating system and version (desktop or mobile): Mac OSX 10.15.6

Reproducible Demo

(Paste the link to an example repo, including a siteConfig.js, and exact instructions to reproduce the issue.)

@sserrata sserrata added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Jul 24, 2020
@patrys
Copy link

patrys commented Jul 24, 2020

We're in the same boat, Navbar's implementation explicitly allows to hide the title by setting it to null but the new config validation logic requires it to be a non-empty string.

@teikjun
Copy link
Contributor

teikjun commented Jul 24, 2020

Thanks, I've opened a PR to allow navbar.title to accept empty string, null or undefined (so the field is optional now).
Feel free to let me know if there's anything else I should change.

@sserrata
Copy link
Contributor Author

Thanks @teikjun, the change looks good to me.

@sserrata
Copy link
Contributor Author

On a related note, I wanted to offer some feedback or a suggestion. The title: Joi.string().required() change wasn't mentioned anywhere in the release notes - at least, I didn't spot it. A question for the maintainers: have you considered adding a tool like commitizen to your toolchain to help with versioning and changelog?

@slorber
Copy link
Collaborator

slorber commented Jul 27, 2020

This is a mistake for which we'll release a fix soon.

@sserrata We use proper commit messages already and lerna-changelog to generate our changelog. This change does appear in the changelog under "validation", as we started validating user-provided site config, theme config, plugin option config in this release.

I'm sorry this has been a breaking change for you in practice. BTW, the title has always been documented as a required field (https://v2.docusaurus.io/docs/docusaurus.config.js#title)

@Josh-Cena Josh-Cena removed the status: needs triage This issue has not been triaged by maintainers label Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants