Skip to content

fix(core): sortRoutes shouldn't have a default baseUrl value, this led to a bug#10054

Merged
slorber merged 2 commits intomainfrom
slorber/fix-route-ordering-regression
Apr 18, 2024
Merged

fix(core): sortRoutes shouldn't have a default baseUrl value, this led to a bug#10054
slorber merged 2 commits intomainfrom
slorber/fix-route-ordering-regression

Conversation

@slorber
Copy link
Collaborator

@slorber slorber commented Apr 18, 2024

Motivation

Fix #10052

Before:

export function sortRoutes(
  routeConfigs: RouteConfig[],
  baseUrl: string = '/',
): void

After:

export function sortRoutes(
  routeConfigs: RouteConfig[],
  baseUrl: string,
): void

The baseUrl should always be explicitly provided, otherwise if we forget it, we have route ordering bugs

Test Plan

unit tests

@slorber slorber added the pr: bug fix This PR fixes a bug in a past release. label Apr 18, 2024
@slorber slorber requested a review from Josh-Cena as a code owner April 18, 2024 11:31
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Apr 18, 2024
@netlify
Copy link

netlify bot commented Apr 18, 2024

[V2]

Name Link
🔨 Latest commit ab341f2
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/66210aaa6baec8000807b38b
😎 Deploy Preview https://deploy-preview-10054--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 configuration.

@github-actions
Copy link

github-actions bot commented Apr 18, 2024

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟠 60 🟢 98 🟢 96 🟢 100 🟠 88 Report
/docs/installation 🟠 60 🟢 96 🟢 100 🟢 100 🟠 88 Report
/docs/category/getting-started 🟠 76 🟢 100 🟢 100 🟢 90 🟠 88 Report
/blog 🟠 70 🟢 100 🟢 100 🟢 90 🟠 88 Report
/blog/preparing-your-site-for-docusaurus-v3 🟠 64 🟢 96 🟢 100 🟢 100 🟠 88 Report
/blog/tags/release 🟠 68 🟢 100 🟢 100 🟠 80 🟠 88 Report
/blog/tags 🟠 77 🟢 100 🟢 100 🟢 90 🟠 88 Report

@github-actions
Copy link

github-actions bot commented Apr 18, 2024

Size Change: 0 B

Total Size: 1.59 MB

ℹ️ View Unchanged
Filename Size
website/.docusaurus/codeTranslations.json 2 B
website/.docusaurus/docusaurus.config.mjs 26.7 kB
website/.docusaurus/globalData.json 91.2 kB
website/.docusaurus/i18n.json 930 B
website/.docusaurus/registry.js 247 kB
website/.docusaurus/routes.js 156 kB
website/.docusaurus/routesChunkNames.json 109 kB
website/.docusaurus/site-metadata.json 2.17 kB
website/build/assets/css/styles.********.css 113 kB
website/build/assets/js/main.********.js 808 kB
website/build/index.html 38 kB

compressed-size-action

@slorber slorber merged commit 1287387 into main Apr 18, 2024
@slorber slorber deleted the slorber/fix-route-ordering-regression branch April 18, 2024 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multi-Instance Routes Not Working in v3.2.1

2 participants