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(core): TypeError: Invalid URL while trying to build with sitemap not set to false #8116 #8164

Closed
wants to merge 2 commits into from

Conversation

a11rew
Copy link

@a11rew a11rew commented Oct 2, 2022

Pre-flight checklist

  • I have read the Contributing Guidelines on pull requests.
  • If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
  • If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

The url field in docusaurus.config.js currently accepts URLs without a preceding protocol causing the downstream sitemap builder to fail when calling new URL() with said invalid URLs.

This PR updates the Joi validation of the field to reject URLs that would cause new URL() to error out.

Implementation

The current validation schema URISchema used in the validation of the url field in docusaurus.config.js allows relative URLs with the {allowRelative: true} config option.

export const URISchema = Joi.alternatives(
Joi.string().uri({allowRelative: true}),

This lets URLs without protocols pass validation and cause errors when constructing URLs. Disabling that option would fix this but URISchema is used in a number of other places, changing it could have unintended consequences.

This PR removes the dependency on URISchema and directly adds Joi .url() validation to the url field without the allowRelative config option. It also adds new URL() construction validation as a fallback to ensure invalid URLs that aren't caught by Joi but error out on new URL() don't make it past validation.

Test Plan

A 'throws for URLs missing protocol' test is added to validate URLs like "docusaurus.io" don't pass config validation.

The inline snapshot for an existing test on the same url field is also updated. Providing a number to the URL field throws a "url" must be a string error instead of a invalid valueerror.

Test links

Deploy preview: https://deploy-preview-_____--docusaurus-2.netlify.app/

Preview of Changes

Descriptive error when invalid URL is provided

Related issues/PRs

Fixes #8116

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Oct 2, 2022
@netlify
Copy link

netlify bot commented Oct 2, 2022

[V2]

Built without sensitive environment variables

Name Link
🔨 Latest commit dd55c72
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/63396db42caf6300083d9e22
😎 Deploy Preview https://deploy-preview-8164--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

github-actions bot commented Oct 2, 2022

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟢 91 🟢 98 🟢 100 🟢 100 🟠 80 Report
/docs/installation 🟠 74 🟢 100 🟢 100 🟢 100 🟢 90 Report

@Josh-Cena
Copy link
Collaborator

Hi, I'm sorry but there's already an open PR for it: #8159 Both are equally close to the finish line, so I'll close this one in favor of that. Thanks a lot for putting your time into it—and feel free to pick up another issue to work on, or review the work in that PR.

@Josh-Cena Josh-Cena closed this Oct 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError: Invalid URL while trying to build with sitemap not set to false
3 participants